search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.79k stars 1.12k forks source link

[docs-infra] Remove no longer needed `next.config` settings #12861

Closed oliviertassinari closed 2 days ago

oliviertassinari commented 2 weeks ago

Propagate https://github.com/mui/material-ui/pull/41901

mui-bot commented 2 weeks ago

Deploy preview: https://deploy-preview-12861--material-ui-x.netlify.app/

Generated by :no_entry_sign: dangerJS against 6f8dbea346a5e9d828f8990ab5037800eff4a369

oliviertassinari commented 1 week ago

We can't merge:

Before: https://master--material-ui.netlify.app/material-ui/getting-started/

SCR-20240420-ufbt

After: https://deploy-preview-12861--material-ui-x.netlify.app/x/introduction/

SCR-20240420-ueyc
github-actions[bot] commented 1 week ago

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Janpot commented 1 week ago

Yep, we had this last week on Toolpad as well. See also the slack thread. I think I will revert https://github.com/mui/mui-toolpad/pull/3301. It's not a great idea to half import from monorepo and half from published @mui/docs. Too many changes go out of sync. We should alias @mui/docs from the monorepo as long as we can't remove the @mui/monorepo dependency as a whole. cc @michaldudak

oliviertassinari commented 1 week ago

Too many changes go out of sync. We should alias @mui/docs from the monorepo as long as we can't remove the @mui/monorepo dependency as a whole

@Janpot On the general migration from GitHub to npm. I don't know, it feels like if we add the alias back, we will never migrate to @mui/docs and other @mui/internal- packages. I have rarely (none that I remember) seen "eventual" integration work, continuous/progressive integration are the ones I have seen work. It seems better to update @mui/monorepo by commits that match with @mui/internal- releases. So if we can release @mui/internal- automatically for each commit, great!

Amazing would be Renovate which automates daily all the @mui/internal-* package if the build is green. For example in https://github.com/mui/mui-x/pull/11303, it feels like we took a good number of shortcuts because we don't know what change broke what or that it's too much to handle at once.

On this specific instance of the bug, it looks to me that it's because we have 3 moving parts (@mui/docs, @mui/monorepo, and @mui/internal-markdown) that if not kept in sync breaks. This is for me the definition of leaky abstractions. So the the npm dependencies looks wrong:

https://github.com/mui/mui-toolpad/blob/558e93f1ff2d4d01874344e8671ea898d101c364/docs/package.json#L29-L31

We have two separate npm packages that are intrinsically linked. A side note, they don't even follow the same versioning. What would make more sense to me is:

Or

It seems I'm stuck with this PR. I can't easily make a change in docs-infra and propagate it yet. I need to wait for new docs infra packages to be released.

Janpot commented 1 week ago

The original plan was

  1. In Toolpad/X: alias the @mui/docs package with the one from the @mui/monorepo github dependency.
  2. Progressively expand the @mui/docs package with docs infra features until the @mui/monorepo dependency becomes obsolete.
  3. Remove the alias for @mui/docs and remove @mui/monorepo dependency.

This strategy has the advantage that we can develop @mui/docs at the same velocity as we update @mui/monorepo. i.e. a single moving target. Unfortunately, we jumped the gun and removed the alias much earlier. I didn't recognize the impact at the time it happened. That's why I'm promoting to revert back https://github.com/mui/mui-toolpad/pull/3301 to get us back on the original plan.

We have two separate npm packages that are intrinsically linked.

This is not a problem as long as the contracts between said packages are respected. They should declare their dependencies and importers should correctly install those dependencies. If the importer decides to alias a bunch of those dependencies to the tip-of-the-tree of a github repository then inconsistencies are expected to happen.

Use GitHub package.json dependency, enforce a single version of everything at all time, one that is continously integrated on Material UI.

It's important to note that the problems we see now with @mui/docs are not new. We already had multiple moving targets. We had the monorepo dependency which was using master branch of core, and that depended on release @mui/ packages. This was already breaking from time to time in Toolpad (e.g. when docs rely on a feature in @mui/material that wasn't released yet).

It seems I'm stuck with this PR. I can't easily make a change in docs-infra and propagate it yet. I need to wait for new docs infra packages to be released.

I suspect if we bring the equivalent of https://github.com/mui/mui-toolpad/pull/3440 to this repo, it will start working again.

I believe ultimately, what we want from a docs infra package is very similar in scope as docusaurus. Personally I'd define the "docs infra" as "docusaurus, but built with/for MUI". It includes (assuming still built on next.js):

I'd envision us to work towards a world where a single dependency on @mui/docs can set up a site where you just have to throw content at. Whether it's @mui/docs or @mui/internal-docs I don't think it should make a lot of difference in the eventual architecture or the way we use it. That difference lies in the velocity of development and the promises we make around breaking changes.

Just throwing something on the table here, but with now at least 5 projects lining up to use this shared docs architecture, and the plans to hire for it. Maybe we should even consider the idea of a public docs package? A public release could act as a forcing function to build something that propagates easily to the projects without compromises.

oliviertassinari commented 2 days ago

@LukasTy Thanks for rebasing, having the codebase dependencies up to date with https://github.com/mui/material-ui is enough to be able to merge this 👍

oliviertassinari commented 2 days ago

It's important to note that the problems we see now with @mui/docs are not new. We already had multiple moving targets. We had the monorepo dependency which was using master branch of core, and that depended on release @mui/ packages.

@Janpot This is of the pros of moving code-infra outside of Material UI, it gives a shorter feedback loop when relying on unreleased stuff. Point recorded to https://www.notion.so/mui-org/docs-infra-Code-location-c292d34ed5b0487a9ade846ef11d4019?pvs=4#af2c9a083adf4045ad8eed9624770516.

a docs infra package is very similar in scope as docusaurus.

This makes the most sense to me. It's like https://shopify.engineering/deconstructing-monolith-designing-software-maximizes-developer-productivity where you release everyone at once, but things are living into compartments, so you can enforce things to be private, so you can get better dependencies control.

Maybe we should even consider the idea of a public docs package? A public release could act as a forcing function to build something that propagates easily to the projects without compromises.

It would set the right hygiene but wouldn't allow us to take shortcuts, it might not be pragmatic. This decision could be more for the person who takes the docs-infra role to make, but even then, should we aspire to do a https://vitepress.dev/guide/what-is-vitepress? I don't know, doesn't feel like it would connect so well with our purpose, it feels like a lot more work to grow this as an open-source project.

cc @mui/code-infra

Janpot commented 1 day ago

It's like https://shopify.engineering/deconstructing-monolith-designing-software-maximizes-developer-productivity where you release everyone at once, but things are living into compartments, so you can enforce things to be private, so you can get better dependencies control.

I think this link nicely illustrates my issues with the github dependency + aliasing approach we take. It disrespects the modular boundaries that we set with pnpm workspaces. It breaks the dependency resolution, and it breaks the explicit contract a package version has between its public API and its version number. The approach works fairly well in a single repo, but scales badly across repositories, or when the scope of the packages widens (e.g. pigment running in untranspiled environments).

The inconvenient truth though is that for dependent repositories to gain back this reliability, core repo will likely have to trade in some of its development velocity.

I don't know, doesn't feel like it would connect so well with our purpose, it feels like a lot more work to grow this as an open-source project.

I don't think we need to necessarily grow this as a public project, but perhaps we could benefit from modularizing it better and pretending a bit more as if it was public. "how would we build this if we had to support it as a public API", then act pragmatically.

I'm also interested in coming up with a good definition of what is "code infra" and what is "docs infra". As that confusion keeps popping up when from time to time. Their naming is a bit ambiguous because calling it "X infra" and "Y infra" implies X and Y are similar things. If I have to define it "docs infra" is docusaurus, and "code infra" is CI, testing, building, hosting, publishing. Perhaps we should consider renaming "code infra" to "devops"? I think what I mostly wanted to allude to is "imagine this as a public package, the scope of the team that works on it, that is docs infra".