[core] Add `pkg.pr.new` publishing

[Aslemammad]

Resolves #42922

Now we only need to install the app so we can have the pr being released!

[mui-bot]

Netlify deploy preview

https://deploy-preview-42984--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against 51495801e83b2cfa183aae90512b1d7b473f8715

[oliviertassinari]

What I'm missing from this PR is why.

To be honest, I don't even understand why CodeSandbox is doing CodeSandbox CI. It's great for us, but seems to mostly be costs for them 😁. The closest thing to what I see as valuable is fixing: https://github.com/stackblitz/core/issues/437#issuecomment-1127076737 but it doesn't seems to need pkg.pr.new.

The cons I see:

• we will have to keep this logic working, like we do with CodeSandbox CI when we make changes to packages
• it's another thing maintainers needs to know about and how it works if developers are using those imports
• it's another source of build failure possible

But maybe CodeSandbox CI is not reliable? I don't know.

---

I tested https://github.com/stackblitz/core/issues/437#issuecomment-1127076737 again, I guess it's not working? https://stackblitz.com/edit/react-raw8yb-b4fq34?file=package.json

[Aslemammad]

The closest thing to what I see as valuable is fixing

I can let the team know so they can fix it. But pkg.pr.new already works with this case.

The cons I see

Yea, it is another workflow! So for instance, if the build steps change, the workflow should be updated for instance, which is something universal across the whole repo though.

I can sort this PR out soon, but if you think closing it make more sense, I'm all there for that, sometimes just adding extra stuff is not worth it. I appreciate the honest feedback here ❤️

[Aslemammad]

It's working now! https://github.com/mui/material-ui/pull/42984/checks?check_run_id=27690943232

[Janpot]

What I'm missing from this PR is why.

Codesandbox had three outages in the last three months. Impacting us more than it should have. Granted, we shouldn't have relied so much on it in our critical paths in the first place. My intent is not to migrate or to allocate a lot of maintenance to pkg.pr.new. But I'd like to know what our options are. If it eats up too much maintenance time, I'm happy to remove it again.

It's working now!

Did a quick install and I noticed that it's not rewriting dependency specifiers for workspace dependencies. e.g. when you install https://pkg.csb.dev/mui/material-ui/commit/5777e630/@mui/material you will notice that its @mui/system dependency version is https://pkg.csb.dev/mui/material-ui/commit/5777e630/@mui/system. Unlike with e.g. https://pkg.pr.new/@mui/material@e0e7bcb where it points to 6.0.0-beta.1. That makes it unusable for us as its not unlikely that changes happen across more than one package and may be incompatible with npm published versions.

[Aslemammad]

That makes it unusable for us as its not unlikely that changes happen across more than one package

Yes, it's already handled in pkg.pr.new but this is likely a bug on our side, sorry for the bad experience, I'll sort it out for you!

[Aslemammad]

I think now it's sorted out, but there's a minor issue I'd like to ask you!

Here's the published package.json (of my fork):

  "dependencies": {
    "@babel/runtime": "^7.24.8",
    "@mui/core-downloads-tracker": "https://pkg.pr.new/Aslemammad/material-ui/@mui/core-downloads-tracker@b29e8b8",
    "@mui/system": "https://pkg.pr.new/Aslemammad/material-ui/@mui/system@b29e8b8",
    "@mui/types": "https://pkg.pr.new/Aslemammad/material-ui/@mui/types@b29e8b8",
    "@mui/utils": "https://pkg.pr.new/Aslemammad/material-ui/@mui/utils@b29e8b8",
    "@popperjs/core": "^2.11.8",
    "@types/react-transition-group": "^4.4.10",
    "clsx": "^2.1.1",
    "csstype": "^3.1.3",
    "prop-types": "^15.8.1",
    "react-is": "^18.3.1",
    "react-transition-group": "^4.4.5"
  },
  "peerDependencies": {
    "@emotion/react": "^11.5.0",
    "@emotion/styled": "^11.3.0",
    "@mui/material-pigment-css": "workspace:^",
    "@types/react": "^17.0.0 || ^18.0.0",
    "react": "^17.0.0 || ^18.0.0",
    "react-dom": "^17.0.0 || ^18.0.0"
  },

As you can see, in pkg.pr.new I decided to not touch peerDependencies, but in this case, I think we might need to do, what do you think? do you think it should be a link like others in dependencies? And would you like us to have a flag --peerDeps that toggles whether we should edit or not?

I would love to hear your thoughts!

[oliviertassinari]

know what our options are.

@Janpot 👍

I can let the team know so they can fix it. But pkg.pr.new already works with this case.

@Aslemammad It seems to work with the runtime side: https://stackblitz.com/edit/vitejs-vite-gmeaac?file=src%2FApp.tsx,package.json,tsconfig.json&terminal=dev. But it's still broken with the older mode that provides a better DX: https://stackblitz.com/edit/react-7qjqad?file=src%2FApp.js,package.json.

I decided to not touch peerDependencies, but in this case, I think we might need to do, what do you think

pnpm would replace this with the version in the workspace. I would expect pkg.pr.new to extend pnpm behavior. This is CodeSandbox CI output, it looks correct:

  "dependencies": {
    "@babel/runtime": "^7.24.8",
    "@mui/core-downloads-tracker": "https://pkg.csb.dev/mui/material-ui/commit/6165feea/@mui/core-downloads-tracker",
    "@mui/system": "https://pkg.csb.dev/mui/material-ui/commit/6165feea/@mui/system",
    "@mui/types": "https://pkg.csb.dev/mui/material-ui/commit/6165feea/@mui/types",
    "@mui/utils": "https://pkg.csb.dev/mui/material-ui/commit/6165feea/@mui/utils",
    "@popperjs/core": "^2.11.8",
    "@types/react-transition-group": "^4.4.10",
    "clsx": "^2.1.1",
    "csstype": "^3.1.3",
    "prop-types": "^15.8.1",
    "react-is": "^18.3.1",
    "react-transition-group": "^4.4.5"
  },
  "peerDependencies": {
    "@emotion/react": "^11.5.0",
    "@emotion/styled": "^11.3.0",
    "@types/react": "^17.0.0 || ^18.0.0",
    "react": "^17.0.0 || ^18.0.0",
    "react-dom": "^17.0.0 || ^18.0.0",
    "@mui/material-pigment-css": "^6.0.0-beta.1"
  },

[Aslemammad]

Resolved now!

In the new release, I messed up the message formatting a bit, but that would be fixed soon.

But anyway, I think this PR is ready!

[Aslemammad]

@LukasTy Thank you so much Lukas, sure! I just merged the next branch. And also resolved the first comment 🙌

[LukasTy]

And also resolved the first comment 🙌

Nitpick: You have removed the comment from where it made sense, as that job calls release:changelog. 🙈

[Aslemammad]

Ohh sorry my bad, just resolved it!

[Janpot]

• Is there any way we can have a single source of truth for which packages to publish? i.e. We already have https://github.com/mui/material-ui/blob/46335634e418cb6347c12b34f090c54534bca926/.codesandbox/ci.json#L5-L26

  Could the pkg-pr-new read this file somehow? e.g. Something along the lines of

  "pkg-pr-new-packages": "node -e \"console.log(require('./.codesandbox/ci.json').packages.join(' '))\"",
  "pkg-pr-new-publish": "pnpm dlx pkg-pr-new publish $(pnpm -s pkg-pr-new-packages)"
  

• I didn't verify, but does pkg.pr.new respect publishConfig.directory?

[Aslemammad]

Is there any way we can have a single source of truth for which packages to publish? i.e. We already have

just resolved this!

I didn't verify, but does pkg.pr.new respect publishConfig.directory?

Since it uses npm/pnpm under the hood for packing, it respects but the issue it creates is that the omitted build directories do not have the url version of the workspace dependencies! That's why we're just targeting /build directories in some of the packages.

[Aslemammad]

Hello everyone, I hope you're doing well!

I wonder if there's anything remaining I can do on this PR, would be happy help further.

[Janpot]

Ok, over the past months we've seen regular instability from codesandboxci. Often some of the packages are reported as published, but the resulting url 404s. Let's advance this PR and replace it if we're satisfied.

The plan is to run both side-by-side for a few weeks in CI, but rewire the codesandbox demos to use pkg.pr.new in PRs. If it runs smooth we can switch off codesandboxci.

@Aslemammad I have a few more comments:

• I noticed that in the templates the package.json is not formatted (compared to the original)
• Is the - run: npm install -g npm@latest line still relevant?
• Because it's much faster to start up, our Stackblitz demos are built on the EngineBlock environment. Unfortunately it seems to not support pkg.pr.new. Would it be possible to add support for pkg.pr.new in the EngineBlock environment?

[Aslemammad]

Hey y'all! thank you so much for trusting us!

I noticed that in the templates the package.json is not formatted (compared to the original)

Let me mention @43081j. I think there might be an edge case with the pr we had for formatting that is breaking in this case.

Is the - run: npm install -g npm@latest line still relevant?

if you use an older version of npm, yes, but if you use the latest (which we shipped a fix for in npm, then no)

Because it's much faster to start up, our Stackblitz demos are built on the EngineBlock environment. Unfortunately it seems to not support pkg.pr.new. Would it be possible to add support for pkg.pr.new in the EngineBlock environment?

For EngineBlock which is not webcontainers, i think esm.sh might help which we have an integration together. let me mention @ije

[Janpot]

For EngineBlock which is not webcontainers, i think esm.sh might help which we have an integration together. let me mention @ije

To note that our demos use the create-react-app template

[43081j]

FYI confbox 0.2.2 fixes the templating problem you had

pkg-types depends on it but should already pull it in, maybe if you npm up confbox or some such thing

[Janpot]

@Aslemammad Further testing uncovered an issue in templates. Take for example https://pkg.pr.new/template/e076f0c6-0ae2-4d11-af6c-313e63c867ab

The /app/favicon file is corrupt, the app errors. The code editor on stackblitz seems to be able to decode it, but when downloading the project locally:

Would be interesting for us to be able to have some basic integration tests on those templates, is that something that could be in-scope for pkg.pr.new. e.g. On each PR, spin up the app in the template and run some integration tests on the result?

[Aslemammad]

The /app/favicon file is corrupt, the app errors. The code editor on stackblitz seems to be able to decode it, but when downloading the project locally:

Oh that's expected :)

because we cannot embed binaries into the template html file that triggers a project instantiation in stackblitz, we have to give a valid link that later stackblitz would try to fetch it instead.

That's why you should face a link instead. Maybe you can fetch it and later run a script that downloads and replaces those files. but those projects should not have an issue in stackblitz editor itself.

I'll talk about this with the team.

[Janpot]

but those projects should not have an issue in stackblitz editor itself.

They do though, in the stackblitz app it errors with:

Error: Image import "next-metadata-image-loader?type=favicon&segment=&basePath=&pageExtensions=tsx&pageExtensions=ts&pageExtensions=jsx&pageExtensions=js!/home/projects/1pd3bf2s--run/src/app/favicon.ico?__next_metadata__" is not a valid image file. The image may be corrupted or an unsupported format.

[Aslemammad]

They do though, in the stackblitz app it errors with:

oh that should not happen! will get back to you.

[Janpot]

Ok, we're moving forward with this, codesandboxci doesn't seem to publish the icons package anymore. e.g. in build they all resolve to a 500.