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

Aslemammad · 2024-07-17T19:41:59Z

Resolves #42922

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

mui-bot · 2024-07-17T19:44:27Z

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 🚫 dangerJS against 4e6dbfd

oliviertassinari · 2024-07-18T23:31:29Z

.github/workflows/ci.yml

@@ -40,6 +40,9 @@ jobs:
      - run: pnpm build:ci
        env:
          NODE_OPTIONS: --max_old_space_size=4096
+
+      - run: pnpm dlx pkg-pr-new publish './packages/*' --template './examples/*' --compact


It looks like this should run in CircleCI. We test the build flow on different OS here.

This will spam notifications and clutter the UI otherwise:

Suggested change

- run: pnpm dlx pkg-pr-new publish './packages/*' --template './examples/*' --compact

- run: pnpm dlx pkg-pr-new publish './packages/*' --template './examples/*' --compact --comment=off

@oliviertassinari Can you install the app on this repo? I don't seem to have write access.

I believe it's already installed.

There would be only one comment by pkg.pr.new and that will be updated again & again! But anyway, I'll sort this one out.

It looks like this should run in CircleCI

Oh, I see, pkg.pr.new is not supported on CircleCI, it needs to be run in github workflows only.

oliviertassinari · 2024-07-19T20:51:30Z

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: stackblitz/core#437 (comment) 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 stackblitz/core#437 (comment) again, I guess it's not working? https://stackblitz.com/edit/react-raw8yb-b4fq34?file=package.json

Aslemammad · 2024-07-20T03:05:32Z

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 · 2024-07-20T03:15:11Z

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

Janpot · 2024-07-20T03:49:35Z

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 · 2024-07-20T04:56:19Z

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 · 2024-07-21T08:57:59Z

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 · 2024-07-21T23:44:54Z

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 · 2024-07-25T12:41:16Z

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!

LukasTy

Thank you for your contribution! 🙏
Could you sync your branch with latest upstream/next to fix CI?

I'm not very familiar with material-ui CI setup, so, I'm not sure I can approve or decline this change, but I'm all for experimenting with it, maybe also in mui-x repo after this and considering swapping the codsandbox for this. 🤔

.github/workflows/ci.yml

Aslemammad · 2024-07-25T18:29:48Z

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

LukasTy · 2024-07-25T20:30:05Z

And also resolved the first comment 🙌

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

This reverts commit 070ba3d.

Aslemammad · 2024-07-25T20:41:49Z

Ohh sorry my bad, just resolved it!

Janpot · 2024-07-26T13:58:16Z

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

material-ui/.codesandbox/ci.json

Lines 5 to 26 in 4633563

    
           "packages": [ 
        
             "packages/markdown", 
        
             "packages/mui-babel-macros", 
        
             "packages/mui-base", 
        
             "packages/mui-codemod", 
        
             "packages/mui-core-downloads-tracker", 
        
             "packages/mui-docs", 
        
             "packages/mui-icons-material", 
        
             "packages/mui-lab", 
        
             "packages/mui-material-nextjs", 
        
             "packages/mui-material", 
        
             "packages/mui-material-pigment-css", 
        
             "packages/mui-private-theming", 
        
             "packages/mui-styled-engine-sc", 
        
             "packages/mui-styled-engine", 
        
             "packages/mui-styles", 
        
             "packages/mui-system", 
        
             "packages/mui-types", 
        
             "packages/mui-utils", 
        
             "packages-internal/docs-utils", 
        
             "packages-internal/scripts", 
        
             "packages-internal/test-utils"

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 · 2024-07-26T17:00:46Z

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 · 2024-08-09T09:55:36Z

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.

init

bad08f7

Aslemammad mentioned this pull request Jul 17, 2024

Continuous Releases for material-ui using pkg.pr.new #42922

Open

aarongarciah requested a review from a team July 17, 2024 20:38

aarongarciah added the core Infrastructure work going on behind the scenes label Jul 17, 2024

aarongarciah changed the title ~~feat: pkg.pr.new~~ [core] pkg.pr.new Jul 17, 2024

add publish

f0cc7ab

oliviertassinari reviewed Jul 18, 2024

View reviewed changes

make it work

e0e7bcb

Aslemammad mentioned this pull request Jul 20, 2024

fix: mui issue with workspace support stackblitz-labs/pkg.pr.new#154

Closed

Aslemammad added 2 commits July 21, 2024 11:53

update

1d75689

less diff

6165fee

add --pnpm

9bbe6cf

Aslemammad mentioned this pull request Jul 24, 2024

feat: --peerDeps stackblitz-labs/pkg.pr.new#156

Merged

LukasTy mentioned this pull request Jul 24, 2024

[charts][ESM] @mui/x-charts does not work with jest mui/mui-x#11568

Closed

try new version of pkg.pr.new

ca3e0ee

LukasTy reviewed Jul 25, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

Aslemammad added 2 commits July 25, 2024 21:55

remove comment

070ba3d

Merge remote-tracking branch 'upstream/next' into feat/pkg.pr.new

6776b01

Revert "remove comment"

e357075

This reverts commit 070ba3d.

revert comment and remove the previous comment

f25da14

LukasTy added the scope: code-infra Specific to the core-infra product label Jul 26, 2024

LukasTy changed the title ~~[core] pkg.pr.new~~ [core] Add pkg.pr.new publishing Jul 26, 2024

LukasTy requested a review from a team July 26, 2024 04:46

Aslemammad added 3 commits July 26, 2024 19:26

single source of truth

629e6f9

single source of truth

92e211e

Merge remote-tracking branch 'upstream/next' into feat/pkg.pr.new

4e6dbfd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Aslemammad commented Jul 17, 2024

mui-bot commented Jul 17, 2024 •

edited

Loading

oliviertassinari Jul 18, 2024

oliviertassinari Jul 18, 2024 •

edited

Loading

Janpot Jul 19, 2024

oliviertassinari Jul 19, 2024

Aslemammad Jul 20, 2024

Aslemammad Jul 20, 2024

oliviertassinari commented Jul 19, 2024 •

edited

Loading

Aslemammad commented Jul 20, 2024

Aslemammad commented Jul 20, 2024

Janpot commented Jul 20, 2024 •

edited

Loading

Aslemammad commented Jul 20, 2024

Aslemammad commented Jul 21, 2024

oliviertassinari commented Jul 21, 2024 •

edited

Loading

Aslemammad commented Jul 25, 2024

LukasTy left a comment

Aslemammad commented Jul 25, 2024

LukasTy commented Jul 25, 2024

Aslemammad commented Jul 25, 2024

Janpot commented Jul 26, 2024 •

edited

Loading

Aslemammad commented Jul 26, 2024

Aslemammad commented Aug 9, 2024

	- run: pnpm dlx pkg-pr-new publish './packages/' --template './examples/' --compact
	- run: pnpm dlx pkg-pr-new publish './packages/' --template './examples/' --compact --comment=off

[core] Add pkg.pr.new publishing #42984

Are you sure you want to change the base?

[core] Add pkg.pr.new publishing #42984

Conversation

Aslemammad commented Jul 17, 2024

mui-bot commented Jul 17, 2024 • edited Loading

Netlify deploy preview

Bundle size report

oliviertassinari Jul 18, 2024

Choose a reason for hiding this comment

oliviertassinari Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Janpot Jul 19, 2024

Choose a reason for hiding this comment

oliviertassinari Jul 19, 2024

Choose a reason for hiding this comment

Aslemammad Jul 20, 2024

Choose a reason for hiding this comment

Aslemammad Jul 20, 2024

Choose a reason for hiding this comment

oliviertassinari commented Jul 19, 2024 • edited Loading

Aslemammad commented Jul 20, 2024

Aslemammad commented Jul 20, 2024

Janpot commented Jul 20, 2024 • edited Loading

Aslemammad commented Jul 20, 2024

Aslemammad commented Jul 21, 2024

oliviertassinari commented Jul 21, 2024 • edited Loading

Aslemammad commented Jul 25, 2024

LukasTy left a comment

Choose a reason for hiding this comment

Aslemammad commented Jul 25, 2024

LukasTy commented Jul 25, 2024

Aslemammad commented Jul 25, 2024

Janpot commented Jul 26, 2024 • edited Loading

Aslemammad commented Jul 26, 2024

Aslemammad commented Aug 9, 2024

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

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

mui-bot commented Jul 17, 2024 •

edited

Loading

oliviertassinari Jul 18, 2024 •

edited

Loading

oliviertassinari commented Jul 19, 2024 •

edited

Loading

Janpot commented Jul 20, 2024 •

edited

Loading

oliviertassinari commented Jul 21, 2024 •

edited

Loading

Janpot commented Jul 26, 2024 •

edited

Loading