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

💦 building widget state #1382

Merged
merged 5 commits into from
Sep 24, 2024
Merged

💦 building widget state #1382

merged 5 commits into from
Sep 24, 2024

Conversation

stevejpurves
Copy link
Contributor

@stevejpurves stevejpurves commented Jul 13, 2024

Aims:

  • extend notebook processing to lift widget state from notebook metadata
  • notebook metadata can be big and should be out-of-band

Todo:

  • first pass
  • don't duplicate metadata (e.g. kernel specs)
  • decide on pattern for out-of-band

Copy link

changeset-bot bot commented Aug 26, 2024

🦋 Changeset detected

Latest commit: a051955

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
myst-cli Patch
mystmd Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things leftover from testing to tidy up, but overall this looks good. Serializing widget state will open up the door to some cool stuff!!

packages/myst-cli/src/process/mdast.ts Outdated Show resolved Hide resolved
packages/myst-cli/src/process/mdast.ts Outdated Show resolved Hide resolved
packages/myst-cli/src/process/notebook.ts Show resolved Hide resolved
packages/myst-cli/src/process/site.ts Outdated Show resolved Hide resolved
packages/myst-cli/src/process/site.ts Outdated Show resolved Hide resolved
/**
* Write widget state to file in public folder
*/
export function transformWidgetsToFile(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Steve and I chatted about this - there is some duplication between this and the notebook outputs caching. These also follow a slightly different pattern than other on-disk caching that is saved to the /cache folder. Definitely potential to normalize and deduplicate code... but for now, I think this standalone function is fine.

@stevejpurves stevejpurves force-pushed the feat/build-widget-state branch from 703e940 to 5d28c85 Compare September 5, 2024 14:03
@@ -254,7 +255,9 @@ export async function transformMdast(
frontmatter,
mdast,
references,
};
widgets,
something: 'else',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a final pass!

packages/myst-cli/src/process/site.ts Outdated Show resolved Hide resolved
packages/myst-cli/src/process/site.ts Outdated Show resolved Hide resolved
@rowanc1 rowanc1 merged commit 9f50eb8 into main Sep 24, 2024
7 checks passed
@rowanc1 rowanc1 deleted the feat/build-widget-state branch September 24, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants