Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🗿always apply overrides and allow for undefined options #257

Merged
merged 9 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/angry-wolves-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@myst-theme/jupyter': patch
'@myst-theme/site': patch
---

Fix busy state on notebook compute toolbar buttons
6 changes: 6 additions & 0 deletions .changeset/six-trees-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@myst-theme/jupyter': patch
'@myst-theme/site': patch
---

Update to latest thebe which includes fixes to jupyterlite and use of the juptyerlab 4.0 components
5 changes: 5 additions & 0 deletions .changeset/twenty-hounds-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@myst-theme/jupyter': patch
---

Always allow `thebe` options to be overridden, even when no key defined in frontmatter.
12,226 changes: 2,223 additions & 10,003 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions packages/jupyter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
"nbtx": "^0.2.3",
"react-syntax-highlighter": "^15.5.0",
"swr": "^2.1.5",
"thebe-core": "^0.3.5",
"thebe-lite": "^0.3.5",
"thebe-react": "^0.3.5",
"thebe-core": "^0.4.3",
"thebe-lite": "^0.4.3",
"thebe-react": "^0.4.3",
"unist-util-select": "^4.0.3"
},
"peerDependencies": {
Expand Down
4 changes: 2 additions & 2 deletions packages/jupyter/src/controls/ArticleCellControls.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useThebeServer } from 'thebe-react';
import { useNotebookExecution } from '../execute/hooks.js';
import { Reset, Run, SpinnerStatusButton } from './Buttons.js';
import { Restart, Run, SpinnerStatusButton } from './Buttons.js';

import { selectAreExecutionScopesBuilding } from '../execute/index.js';

Expand Down Expand Up @@ -47,7 +47,7 @@ export function ArticleResetNotebook({ id }: { id: string }) {
const { ready, notebookIsResetting, notebookIsBusy, reset } = useNotebookExecution(id);
if (!ready) return null;
return (
<Reset
<Restart
ready={ready}
resetting={notebookIsResetting}
disabled={notebookIsBusy}
Expand Down
21 changes: 10 additions & 11 deletions packages/jupyter/src/controls/Buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,12 @@ function SpinnerButton({
return (
<div className="relative flex text-sm">
<button
className={classNames(
'cursor-pointer text-gray-700 dark:text-white active:text-green-700 hover:opacity-100',
{
'opacity-10 hover:opacity-10': busy,
'opacity-70': !busy,
},
)}
className={classNames(' text-gray-700 dark:text-white active:text-green-700 ', {
'opacity-10 hover:opacity-10': busy,
'opacity-70': !busy && !disabled,
'cursor-pointer hover:opacity-100': !disabled,
'cursor-not-allowed opacity-10 hover:opacity-10': disabled,
})}
disabled={disabled || !ready || busy}
onClick={() => onClick()}
title={title ?? 'run all cells'}
Expand Down Expand Up @@ -290,7 +289,7 @@ export function ReRun({
);
}

export function Reset({
export function Restart({
ready,
resetting,
disabled,
Expand Down Expand Up @@ -330,9 +329,9 @@ export function Clear({
}) {
return (
<button
className={classNames('flex text-gray-700 dark:text-white opacity-70 ', {
'cursor-not-allowed': disabled || !ready,
'active:text-green-700 hover:opacity-100 cursor-pointer': !disabled,
className={classNames('flex text-gray-700 dark:text-white', {
'cursor-not-allowed opacity-10': disabled || !ready,
'active:text-green-700 opacity-70 hover:opacity-100 cursor-pointer': !disabled,
})}
disabled={disabled || !ready}
onClick={() => onClick()}
Expand Down
5 changes: 3 additions & 2 deletions packages/jupyter/src/controls/NotebookToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { useThebeServer } from 'thebe-react';
import { PowerIcon } from '@heroicons/react/24/outline';
import { Spinner } from './Spinner.js';
import { Clear, Launch, Reset, Run } from './Buttons.js';
import { Clear, Launch, Restart, Run } from './Buttons.js';
import classNames from 'classnames';

export function NotebookToolbar({ showLaunch = false }: { showLaunch?: boolean }) {
Expand Down Expand Up @@ -81,10 +81,11 @@ export function NotebookToolbar({ showLaunch = false }: { showLaunch?: boolean }
/>
)}
{ready && (
<Reset
<Restart
ready={ready}
resetting={busy.page(slug, 'reset')}
onClick={handleReset}
disabled={busy.page(slug, 'execute')}
title="Reset notebook and restart kernel"
/>
)}
Expand Down
8 changes: 8 additions & 0 deletions packages/jupyter/src/execute/leaf.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ export function SessionStarter({
if (!core || !server || scope.session || lock.current) return;
lock.current = true;
console.debug(`Jupyter: Starting session for ${pageSlug}-${notebookSlug} at ${location}`);
if (location === undefined) {
console.warn(
'Article/Notebook json is missing the location field, this maybe break notebook execution when located outside of the root folder',
);
}

server.listRunningSessions().then((sessions) => {
console.debug('Jupyter: running sessions', sessions);
Expand All @@ -130,9 +135,12 @@ export function SessionStarter({
// we need to replace the filename with one based on the page slug and notebook slug
// in order to allow for multiple independent sessions of the same notebook
let path = `/${pageSlug}-${notebookSlug}.ipynb`;
console.debug('session starter path:', path);
const match = location?.match(/(.*)\/.*.ipynb$/) ?? null;
if (match) {
console.debug('session starter match:', match);
path = `${match[1]}/${pageSlug}-${notebookSlug}.ipynb`;
console.debug('session starter path (modified):', path);
}

const existing = sessions.find((s) => s.path === path);
Expand Down
10 changes: 5 additions & 5 deletions packages/jupyter/src/execute/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
selectSessionsToStart,
} from './selectors.js';
import { MdastFetcher, NotebookBuilder, ServerMonitor, SessionStarter } from './leaf.js';
import { useCanCompute } from '../providers.js';

Check warning on line 15 in packages/jupyter/src/execute/provider.tsx

View workflow job for this annotation

GitHub Actions / lint

'useCanCompute' is defined but never used
import type { Thebe } from 'myst-frontmatter';

Check warning on line 16 in packages/jupyter/src/execute/provider.tsx

View workflow job for this annotation

GitHub Actions / lint

'Thebe' is defined but never used
import type { GenericParent } from 'myst-common';

export interface ExecuteScopeType {
Expand Down Expand Up @@ -105,10 +106,9 @@
*/
export function ExecuteScopeProvider({
children,
enable,
contents,
}: React.PropsWithChildren<{ contents: ArticleContents }>) {
const canCompute = useCanCompute();

}: React.PropsWithChildren<{ enable: boolean; contents: ArticleContents }>) {
// compute incoming for first render
const computables: Computable[] = listComputables(contents.mdast);

Expand Down Expand Up @@ -148,14 +148,14 @@

const memo = React.useMemo(
() => ({
canCompute,
canCompute: enable,
slug: contents.slug,
location: contents.location,
state,
dispatch,
idkmap: idkmap.current,
}),
[state, contents.slug],
[state, contents.slug, enable],
);

if (typeof window !== 'undefined') {
Expand Down
3 changes: 1 addition & 2 deletions packages/jupyter/src/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,7 @@ export function useLaunchBinder() {
if (userServerUrl && location) {
// add the location to the url pathname
const url = new URL(userServerUrl);
if (url.pathname.endsWith('/')) url.pathname = url.pathname.slice(0, -1);
url.pathname = `${url.pathname}/lab/tree${location}`;
url.pathname = `${url.pathname}lab/tree${location}`.replace(/\/+/g, '/');
userServerUrl = url.toString();
}
return userServerUrl;
Expand Down
38 changes: 16 additions & 22 deletions packages/jupyter/src/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,29 @@ import React, { useContext } from 'react';
import { ThebeServerProvider } from 'thebe-react';
import { type ExtendedCoreOptions, thebeFrontmatterToOptions } from './utils.js';
import type { GenericParent } from 'myst-common';
import type { SiteManifest } from 'myst-config';
import type { RepoProviderSpec } from 'thebe-core';
import type { ManifestProject } from '@myst-theme/providers';
import { useProjectManifest } from '@myst-theme/providers';

function makeThebeOptions(
siteManifest: SiteManifest | undefined,
optionsOverrideFn = (opts: ExtendedCoreOptions) => opts,
project: ManifestProject,
optionsOverrideFn = (opts?: ExtendedCoreOptions) => opts,
): {
options?: ExtendedCoreOptions;
githubBadgeUrl?: string;
binderBadgeUrl?: string;
} {
if (!siteManifest) return {};
// TODO there may be multiple projects?
// useProjectManifest?
const mainProject = siteManifest?.projects?.[0];
const thebeFrontmatter = mainProject?.thebe;
const githubBadgeUrl = mainProject?.github;
const binderBadgeUrl = mainProject?.binder;
if (!project) return {};
const thebeFrontmatter = project?.thebe;
const githubBadgeUrl = project?.github;
const binderBadgeUrl = project?.binder;
const optionsFromFrontmatter = thebeFrontmatterToOptions(
thebeFrontmatter,
githubBadgeUrl,
binderBadgeUrl,
);

let options = optionsFromFrontmatter;
if (options) options = optionsOverrideFn(options);
const options = optionsOverrideFn(optionsFromFrontmatter);

return {
options,
Expand All @@ -46,29 +43,26 @@ type ThebeOptionsContextType = {
const ThebeOptionsContext = React.createContext<ThebeOptionsContextType | undefined>(undefined);

export function ConfiguredThebeServerProvider({
siteManifest,
optionOverrideFn,
customRepoProviders,
children,
}: React.PropsWithChildren<{
siteManifest?: SiteManifest;
optionOverrideFn?: (opts: ExtendedCoreOptions) => ExtendedCoreOptions;
optionOverrideFn?: (opts?: ExtendedCoreOptions) => ExtendedCoreOptions | undefined;
customRepoProviders?: RepoProviderSpec[];
}>) {
const project = useProjectManifest();
const thebe = React.useMemo(
() => makeThebeOptions(siteManifest, optionOverrideFn),
[siteManifest, optionOverrideFn],
() => (project ? makeThebeOptions(project, optionOverrideFn) : undefined),
[project, optionOverrideFn],
);

if (!siteManifest) return <>{children}</>;

return (
<ThebeOptionsContext.Provider value={thebe}>
<ThebeServerProvider
connect={false}
options={thebe.options}
useBinder={thebe.options?.useBinder ?? false}
useJupyterLite={thebe.options?.useJupyterLite ?? false}
options={thebe?.options}
useBinder={thebe?.options?.useBinder ?? false}
useJupyterLite={thebe?.options?.useJupyterLite ?? false}
customRepoProviders={customRepoProviders}
>
<>{children}</>
Expand Down
1 change: 1 addition & 0 deletions packages/providers/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ export * from './site.js';
export * from './tabs.js';
export * from './xref.js';
export * from './types.js';
export * from './project.js';
19 changes: 19 additions & 0 deletions packages/providers/src/project.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React, { useContext } from 'react';
import type { SiteManifest } from 'myst-config';
import { useSiteManifest } from './site.js';

export type ManifestProject = Required<SiteManifest>['projects'][0];

const ProjectContext = React.createContext<ManifestProject | undefined>(undefined);

export function ProjectProvider({ children }: { children: React.ReactNode }) {
const config = useSiteManifest();
return (
<ProjectContext.Provider value={config?.projects?.[0]}>{children}</ProjectContext.Provider>
);
}

export function useProjectManifest() {
const config = useContext(ProjectContext);
return config;
}
2 changes: 1 addition & 1 deletion packages/site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"nbtx": "^0.2.3",
"node-cache": "^5.1.2",
"node-fetch": "^2.6.11",
"thebe-react": "^0.3.5",
"thebe-react": "^0.4.3",
"unist-util-select": "^4.0.1"
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/site/src/pages/Article.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const ArticlePage = React.memo(function ({
frontmatter={article.frontmatter}
>
<BusyScopeProvider>
<ExecuteScopeProvider contents={article}>
<ExecuteScopeProvider enable={canCompute} contents={article}>
{!hide_title_block && (
<FrontmatterBlock
kind={article.kind}
Expand Down
14 changes: 10 additions & 4 deletions packages/site/src/pages/Root.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import type { SiteManifest } from 'myst-config';
import type { SiteLoader } from '@myst-theme/common';
import { BaseUrlProvider, SiteProvider, Theme, ThemeProvider } from '@myst-theme/providers';
import {
BaseUrlProvider,
ProjectProvider,
SiteProvider,
Theme,
ThemeProvider,
} from '@myst-theme/providers';
import {
Links,
LiveReload,
Expand Down Expand Up @@ -67,9 +73,9 @@ export function Document({
<BaseUrlProvider baseurl={baseurl}>
<ThebeBundleLoaderProvider loadThebeLite={loadThebeLite} publicPath={baseurl}>
<SiteProvider config={config}>
<ConfiguredThebeServerProvider siteManifest={config}>
{children}
</ConfiguredThebeServerProvider>
<ProjectProvider>
<ConfiguredThebeServerProvider>{children}</ConfiguredThebeServerProvider>
</ProjectProvider>
</SiteProvider>
</ThebeBundleLoaderProvider>
</BaseUrlProvider>
Expand Down
1 change: 1 addition & 0 deletions themes/article/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ yalc.lock
/public/build
/public/*thebe*
/public/service-worker-*.js
/public/service-worker.js
/api/*
/build/*

Expand Down
4 changes: 2 additions & 2 deletions themes/article/app/routes/$.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export function Article({
frontmatter={article.frontmatter}
>
<BusyScopeProvider>
<ExecuteScopeProvider contents={article}>
<ExecuteScopeProvider enable={canCompute} contents={article}>
{!hideTitle && <FrontmatterBlock frontmatter={{ title, subtitle }} className="mb-5" />}
{!hideOutline && (
<div className="sticky top-0 z-10 hidden h-0 pt-2 ml-10 col-margin-right lg:block">
Expand Down Expand Up @@ -186,7 +186,7 @@ export function ArticlePage({ article }: { article: PageLoader }) {
frontmatter={article.frontmatter}
>
<BusyScopeProvider>
<ExecuteScopeProvider contents={article}>
<ExecuteScopeProvider enable={canCompute} contents={article}>
<ArticleHeader frontmatter={project as any}>
<div className="pt-5 md:self-center h-fit lg:pt-0 col-body lg:col-margin-right-inset">
<Downloads />
Expand Down
1 change: 1 addition & 0 deletions themes/book/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ yalc.lock
/public/build
/public/*thebe*
/public/service-worker-*.js
/public/service-worker.js
/api/*
/build/*

Expand Down