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

☣️ Refactor transformMdast and postProcessMdast to a single pipeline #1699

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

agoose77
Copy link
Contributor

This spike investigates fusing our single and multi-document pipelines. The motivation for this follows from previous conversations at SciPy 2024 about making more responsive MyST start builds, and capturing transform dependencies.

We originally discussed building MyST transforms as a directional graph. This PR is much less ambitious; it's not clear that a directional graph is necessarily desirable; MyST would become much harder to reason about if we can't treat it as a linear series of transforms.

There were other reasons not do go as far as a graph, but I can't remember them at this very moment.

  1. Support fine-grained before and after constraints on plugins
  2. Reason about MyST "pipeline" in one place
  3. Prepare for customisable pipelines e.g. fast and slow passes

We did not have to fuse the pre and post phases; that was a choice to put logic in one place. The rationale is that we can treat the need perform referencing across pages as a "sync" point for builds within a single-document pipeline.

Copy link

changeset-bot bot commented Dec 12, 2024

⚠️ No Changeset found

Latest commit: 11fc17a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@agoose77
Copy link
Contributor Author

I'm breaking for Christmas, but I will take notes here of what is left to do!

  • Simplify set-shared-state & reference-resolution -- this obfuscates parsing of shared states and then generation of MultiPageReferenceResolver. This can be done in a single blocking promise, and should be written somewhere for index generation to use.
  • Move body of indexingPromise.then() to transformMdast, and introduce a blocking sync point after?
  • Maybe add implementation of barrier makeBarrier(name: string, impl: () => void) to allow barrier body (e.g. cross-reference generation) to be defined in transformMdast?
  • Change transformMdast to build a pipeline, and have the caller invoke it.
  • Change finalizeMdast to build a pipeline, and have the caller invoke it.

@rowanc1
Copy link
Member

rowanc1 commented Jan 6, 2025

This needs to be updated when we merge #1727, the gates plugin needs to happen before the blocks.

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