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

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

Merged
merged 2 commits into from
May 1, 2024

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari added the dependencies Update of dependencies label Apr 19, 2024
@mui-bot
Copy link

mui-bot commented Apr 19, 2024

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

Generated by 🚫 dangerJS against 6f8dbea

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 20, 2024

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 20, 2024
Copy link

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

@Janpot
Copy link
Member

Janpot commented Apr 21, 2024

Yep, we had this last week on Toolpad as well. See also the slack thread.
I think I will revert mui/toolpad#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
Copy link
Member Author

oliviertassinari commented Apr 21, 2024

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 #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:

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

Or

  • Rename @mui/docs to @mui/internal-docs. One is for public use cases, the other for internal, we are mixing things
  • Make @mui/internal-docs the only public npm package for docs infra. Maybe we truly need @mui/internal-markdown to exist for code-infra, but we don't want it to be possible for docs-infra users to use different versions, so we want it private
  • If we can't make it private, and for the others, we want everything with the same npm package version. Keeping us as close as possible to one codebase (the sum of the repositories), continuously integrated.

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
Copy link
Member

Janpot commented Apr 22, 2024

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 mui/toolpad#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 mui/toolpad#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):

  • demo components
  • next.js plugin
  • markdown loader
  • _app.js/_document.js components
  • all dependencies used by docs infra included (importer doesn't have to hack around copying all dependencies to their project's package.json and keep them in sync)

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.

@LukasTy LukasTy added core Infrastructure work going on behind the scenes and removed dependencies Update of dependencies labels Apr 30, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 30, 2024
@LukasTy LukasTy changed the title [core] Update monorepo [core] Remove no longer needed next.config settings Apr 30, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

LGTM.
@oliviertassinari I've updated the PR and changed the title. Does it make sense to you?
The version of monorepo that is used already has the settings in withDocsInfra. 😉

@oliviertassinari oliviertassinari added the scope: docs-infra Specific to the docs-infra product label May 1, 2024
@oliviertassinari oliviertassinari changed the title [core] Remove no longer needed next.config settings [docs-infra] Remove no longer needed next.config settings May 1, 2024
@oliviertassinari oliviertassinari merged commit f6a832d into mui:master May 1, 2024
20 checks passed
@oliviertassinari oliviertassinari deleted the upgrade-monorepo-v2 branch May 1, 2024 13:32
@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 1, 2024

@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
Copy link
Member Author

oliviertassinari commented May 1, 2024

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 on 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
Copy link
Member

Janpot commented May 2, 2024

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".

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 4, 2024

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.

If you mean in the sense that properly abstracting pieces of code, avoiding leaking abstraction takes more time, then I agree. Otherwise, I don't see why we have to give up on development velocity.

Their naming is a bit ambiguous because calling it "X infra" and "Y infra" implies X and Y are similar things.

We have infra as a label for the rest, for example, Michal with mui/mui-public#149 would be working on support/product infra. DevOps feels like it has a too-strong operations and too engineering component. In our case, code-infra feels closer to product ops.

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".

Right, so it feels like, the problem we need to fix is enforced boundaries in the code, no surprise side effects:
SCR-20240504-uldg

The problem we don't want to fix is independent deployment units:

SCR-20240504-ulpj

DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024
Co-authored-by: Lukas <llukas.tyla@gmail.com>
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
Co-authored-by: Lukas <llukas.tyla@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants