[code-infra] Prepare the markdown package for publishing #41240
Conversation
Netlify deploy previewhttps://deploy-preview-41240--material-ui.netlify.app/ Bundle size report |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| const importPaths = fileContent.match(/'.*\?@mui\/markdown'/g); | ||
| const importPaths = fileContent.match(/'.*\?muiMarkdown'/g); |
There was a problem hiding this comment.
This change broke pnpm docs:link-check in the other repositories that keep importing markdown files with
?@mui/markdown. I pushed 8f9cf1f to at minimum have no breaking changes.
There was a problem hiding this comment.
Which repositories use ?@mui/markdown still? I was under the impression we replaced it everywhere.
First time I see this code but as a suggestion: I feel I think it could make more sense to search for an import to a .mdx? file instead and ignore the url query altogether:
const match = /import .* from \'(.+.mdx?)(\?.*)?\'/.exec(filecontent)
const mdPath = match[1]I think it better covers the intent. ?@muiMarkdown is a MUI proprietary thing and I'd rather see us remove it in favor for just a bare .md specifier.
There was a problem hiding this comment.
It seems that Base UI uses @mui/markdown - my oversight. But we don't need to change this, as we're most likely moving away from this markdown loader in favor of MDX.
There was a problem hiding this comment.
Which repositories use ?@mui/markdown still?
I have configured VS Code to have all the repositories for search
Yes, I was thinking of Base UI.
as we're most likely moving away from this markdown loader in favor of MDX.
To make sure we can continue to do extra processing at built-time before this reaches the client bundle. Also the maintenance of MDX looks a bit dead https://github.com/mdx-js/mdx/graphs/contributors, so to be careful that this solves more problems we are facing than it creates 😄.
There was a problem hiding this comment.
make sure we can continue to do extra processing at built-time before this reaches the client bundle
Sure, nothing has changed in this regard.
Also the maintenance of MDX looks a bit dead https://github.com/mdx-js/mdx/graphs/contributors, so to be careful that this solves more problems we are facing than it creates 😄.
Interesting. Considering their number of downloads (the same order of magnitude as Material UI), I'm surprised how few issues they have. Perhaps it just works and doesn't need a lot of maintenance.
There was a problem hiding this comment.
@michaldudak As long as we can implement stuff like #43562 outside of the bundle we send to browsers, happy.
Interesting. Considering their number of downloads (the same order of magnitude as Material UI), I'm surprised how few issues they have. Perhaps it just works and doesn't need a lot of maintenance.
What I see: with a custom stack we can quickly iterate on it to make changes, with MDX, it iteration cycles might be slower. So if we trade speed, I imagine it's because we want other features in exchange. I'm not familiar with the advantages that MDX has. I always assumed that we were OK without it.
There was a problem hiding this comment.
MDX works as a series of AST transformations. Much like babel but it also converts between multiple ST types. Some(/all?) of those transformations can be manipulated with plugins. I had a chance to play around with this a bit in https://github.com/Janpot/rsc-playground/blob/master/src/lib/remarkPlugin.mjs.
It may potentially have a bit higher learning curve, but it feels powerful. e.g. the way we currently do callouts makes it impossible to nest markdown inside.
There was a problem hiding this comment.
As long as we can implement stuff like #43562 outside of the bundle we send to browsers
Yup, this shouldn't be a problem at all.
Some(/all?) of those transformations can be manipulated with plugins.
I actually implemented a plugin to extract href attributes of all links for the updated broken link validator - it's pretty easy (as long as you're familiar with traversing ASTs).
Renamed the @mui/markdown to @mui/internal-markdown and prepared it for publishing.
I had to modify the package itself to make it independent from the location it's stored.
Most of the changed files are related to the rename (they are all in a single commit, so they can be skipped during review).
The changes in docs files are not strictly necessary (as the imported path query string is also configurable in webpack settings), but I opted for consistency.
X integration: mui/mui-x#12188
Toolpad integration: mui/toolpad#3235