:sparkles: Add `text/markdown` MIME renderer by agoose77 · Pull Request #102 · jupyter-book/jupyterlab-myst

agoose77 · 2023-02-27T14:57:34Z

Fixes #100 by overwriting the default text/markdown MIME renderer with a MyST enabled renderer.

github-actions · 2023-02-27T14:57:46Z

👈 Launch a Binder on branch agoose77/jupyterlab-myst/feat-mime-renderer

agoose77 · 2023-02-27T14:59:12Z

@rowanc1 deleting the yarn lockfile leads to the build failing. I can't figure out exactly what the error is, hoping you'll know.

rowanc1

Adding a few thoughts as I look through this!

rowanc1 · 2023-02-27T16:01:43Z

+    this.resolver = options.resolver;
+    this.linkHandler = options.linkHandler;
+    this.node.dataset['mimeType'] = MIME_TYPE;
+    this.addClass('myst-RenderedMySTMarkdown');


There is some css that requires this to be wrapped in a top-level myst, we can change the name of that, but it will need it to be the same as the cell, or added in.

rowanc1 · 2023-02-27T16:03:04Z

+    const { frontmatter: frontmatterRaw } = getFrontmatter(
+      // This is a bit weird, but if there is a YAML block in the first cell, this is where it will be.
+      mdast,
+      {
+        removeYaml: true,
+        removeHeading: true
+      }
+    );


My understanding is that this is for inline markdown? I think that we shouldn't parse the frontmatter unless it is specifically the first cell of a notebook.

rowanc1 · 2023-02-27T16:04:42Z

+            >
+              <TabStateProvider>
+                <ReferencesProvider
+                  references={references}


Is this intended to share the state with other cells, e.g. can you reference an equation in the notebook with this?

It that is the case, then this needs to be the shared references, which gets a bit tricky (maybe?)

Yes it does! When I wrote this PR initially, I just wanted to get the majority of the boilerplate on the page. The semantics are probably in need of some work :)

agoose77 · 2023-02-27T20:46:16Z

@rowanc1 this now builds, and renders, but it's a bit broken:

styles seem a bit borked
no scrolling
front matter seems duplicated

I just wired everything together (I have no high-level understanding of the details of the AST transform/rendering), so I'm not surprised. Are you happy to take over this PR when you're next free (no rush)?

tavin · 2023-05-01T18:25:06Z

@agoose77 @rowanc1 I was trying to grok this PR over the weekend... does it need to be rebased off the latest main branch?

Addresses error with nth-check source-map parsing: webpack-contrib/source-map-loader#186

tavin · 2023-05-12T11:35:11Z

+    this.resolver = options.resolver;
+    this.linkHandler = options.linkHandler;
+    this.node.dataset['mimeType'] = MIME_TYPE;
+    this.addClass('jp-RenderedMySTMarkdown');


should this be jp-MarkdownOutput? or both?

I'm avoiding using the Jupyter tags, as they bring their own assumptions that might not reflect ours. We don't use the jp tags elsewhere, but if we need to, we can! For now, assuming not.

ok, I was comparing with the CSS classes used on the notebook side (that make styles and scrolling work) -- I haven't tried your latest changes yet

Ah, I see! I've fixed the scrolling (setting overflow rules, although it's currently a private variable - we should use our own), and the styles look correct to me, now.

tavin · 2023-05-15T09:12:25Z

@rowanc1 deleting the yarn lockfile leads to the build failing. I can't figure out exactly what the error is, hoping you'll know.

@agoose77 I just came across this old remark of yours -- I think the answer is indeed pinning the @jupyterlab deps, either using resolutions or else by eschewing the caret i.e. 3.5.1 instead of ^3.5.1 ... 3.4.7 will downgrade the yarn lockfile at this point, but maybe that's okay.

agoose77 · 2023-05-15T09:15:02Z

I think the answer is indeed pinning the @jupyterlab deps, either using resolutions or else by eschewing the caret i.e. 3.5.1 instead of ^3.5.1

We want to support all versions of JupyterLab greater than our lower bound. Therefore, I think we don't even want to pin these deps during development, as it means we'll catch these kinds of failures before someone installs the package in whichever JupyterLab is misbehaving.

Although we could probably go lower than 3.4.7, that's the minimum version that our PyPI package supports, so it's already "in the wild". If there are build failures with a specific version of JupyterLab, we should fix those directly. When the extension is installed in JupyterLab, it will provide its own versions of these packages, after all!

tavin · 2023-05-15T09:18:41Z

When the extension is installed in JupyterLab, it will provide its own versions of these packages, after all!

True. Should the yarn lockfile be removed from git and gitignored? Otherwise it somehow still seems wrong to me to have all these 3.6.3 versions in the yarn lockfile.

Edit 1: Ah looks like you fixed that (except for markdownviewer).

Edit 2:

We want to support all versions of JupyterLab greater than our lower bound. Therefore, I think we don't even want to pin these deps during development, as it means we'll catch these kinds of failures before someone installs the package in whichever JupyterLab is misbehaving.

@agoose77 I like that philosophy but I think it won't happen unless the yarn lockfile is actually removed from version control?

agoose77 · 2023-05-15T09:37:31Z

@agoose77 I like that philosophy but I think it won't happen unless the yarn lockfile is actually removed from version control?

Yes, for the most part this is true. In general, my aversion to resolutions is that they're used to fix broken package.jsons rather than specify our own constraints. However, I'm actually not aware of any projects that drop their yarn.lock, meaning that existing extensions are all working on a single lockfile, and hoping that semver protects them from breakages. This is interesting, and I suppose the main benefit is much speedier CI!

As such, unless @rowanc1 has any reservations, I'd prefer to continue working with a central lockfile that we'll keep up to date, and we'll open bug reports if something breaks within our constraints.

tavin · 2023-05-15T09:51:41Z

As such, unless @rowanc1 has any reservations, I'd prefer to continue working with a central lockfile that we'll keep up to date, and we'll open bug reports if something breaks within our constraints.

So upgrading most/all of the libraries in the yarn lockfile was the intention (I now see this has been done on the main branch)? Sorry I did not realize that's what you wanted, it's a different approach than what I had in mind (i.e. keeping them locked to the lower bound). Thanks for the background!

agoose77 · 2023-05-15T11:42:36Z

Sorry I did not realize that's what you wanted

No need to apologise for a good discussion.

I'm not a JS developer by trade, so take this with a pinch of salt. My contention is that the actual locked versions don't matter, as long as they work. What matters is our package.json, which describes the set of versions of our dependencies that we support. When it comes to the built package, JupyterLab (via module federation) deduplicates dependencies and applies its own rules to determine which versions of our dependencies we ultimately load.

So, I'm viewing the lockfile as "ensure everyone's working with the same dependencies", which is not entirely useful as we're developing a library/plugin rather than a standalone application, but is the typical workflow.

tavin · 2023-05-15T11:57:14Z

@agoose77 My concern was basically being unable to build the project without a magic lockfile :)
I think I'm happy if either the lockfile is pinned to the lower bounds or upgraded all the way to the latest. So I'm good now.

agoose77 · 2023-05-15T12:06:43Z

To be clear on my position here —

build the project without a magic lockfile :)

should never fail either; it should be that whatever lockfile you have, the build succeeds. If that fails, we don't support what we say we support, and should fix it!

In any case, let's see how things go :)

rowanc1 · 2023-05-15T23:29:57Z

Just a quick note to apologize for my lack of attention over in this repo! @tavin it has been great working with you over in the theme, and exciting to see that the work can come over here easily with proofs and all the style updates.

I am on vacation the next week and a bit, so hopefully won't be checking GitHub that much. Hoping when I get back to give this repo a bit more of my time!

agoose77 · 2023-05-30T11:16:33Z

In the interest of getting JupyterLab 4.x working, I'm going to merge this. It appears to work!

rowanc1 · 2023-05-30T16:33:33Z

Excited to try this!

rowanc1 reviewed Feb 27, 2023

View reviewed changes

agoose77 changed the title ~~wip: add mime renderer~~ feat: add text/markdown MIME renderer Feb 27, 2023

agoose77 mentioned this pull request Mar 22, 2023

Export the myst markdown renderer as a widget #126

Open

tavin reviewed May 2, 2023

View reviewed changes

Comment thread package.json

tavin reviewed May 2, 2023

View reviewed changes

Comment thread yarn.lock

tavin mentioned this pull request May 9, 2023

trim deps and upgrade to @jupyterlab 3.6 #137

Closed

agoose77 and others added 6 commits May 12, 2023 09:49

wip: add mime renderer

5ee1958

wip: build out mime renderer

c1e64f4

chore: revert change

28e87ab

chore: fix build error by updating source-map-loader

24e75b4

chore: revert to jp- class structure

453156c

fix build errors

8bff032

agoose77 force-pushed the feat-mime-renderer branch from 0ce22a0 to 8bff032 Compare May 12, 2023 09:17

agoose77 added 9 commits May 12, 2023 10:17

chore: simplify dependencies

d10e997

chore: drop local files

8337fed

feat!: revert to existing style names

8987ffc

fix: update source-map-loader

5d4c6be

Addresses error with nth-check source-map parsing: webpack-contrib/source-map-loader#186

wip: examples

0d8cbe3

fix: styling of rendered markdown

ba97846

chore: fix styling formatting

39573ce

chore: use shorthand styling

250e22d

chore: run prettier

4c49c1f

tavin reviewed May 12, 2023

View reviewed changes

agoose77 changed the title ~~feat: add text/markdown MIME renderer~~ ✨ Add text/markdown MIME renderer May 12, 2023

Merge branch 'main' into feat-mime-renderer

43f5274

agoose77 added the enhancement New feature or request label May 15, 2023

refactor: cleanups & backports

0886b02

agoose77 requested a review from rowanc1 May 15, 2023 09:39

agoose77 mentioned this pull request May 16, 2023

🪐 Support for JupyterLab 4.x #142

Closed

agoose77 marked this pull request as ready for review May 22, 2023 08:35

agoose77 added 3 commits May 22, 2023 09:35

Merge branch 'main' into feat-mime-renderer

3e57f42

fix: constrain rendermime-interfaces

f95275e

fix: change to footnotes

f70e198

agoose77 merged commit daab803 into jupyter-book:main May 30, 2023

agoose77 deleted the feat-mime-renderer branch May 30, 2023 11:16

rowanc1 mentioned this pull request May 30, 2023

Synced Rendering Capability on JupyterLab #144

Closed

tavin mentioned this pull request Jun 13, 2023

Consider how users can extend roles/directives #45

Open

Uh oh!

Conversation

agoose77 commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 27, 2023

Uh oh!

agoose77 commented Feb 27, 2023

Uh oh!

rowanc1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agoose77 commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tavin commented May 1, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tavin commented May 15, 2023

Uh oh!

agoose77 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tavin commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agoose77 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tavin commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agoose77 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tavin commented May 15, 2023

Uh oh!

agoose77 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rowanc1 commented May 15, 2023

Uh oh!

agoose77 commented May 30, 2023

Uh oh!

rowanc1 commented May 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

agoose77 commented Feb 27, 2023 •

edited

Loading

agoose77 commented Feb 27, 2023 •

edited

Loading

agoose77 commented May 15, 2023 •

edited

Loading

tavin commented May 15, 2023 •

edited

Loading

agoose77 commented May 15, 2023 •

edited

Loading

tavin commented May 15, 2023 •

edited

Loading

agoose77 commented May 15, 2023 •

edited

Loading

agoose77 commented May 15, 2023 •

edited

Loading