graphiql
graphiql copied to clipboard
yarn => pnpm
ref: https://github.com/graphql/graphiql/issues/2185
⚠️ No Changeset found
Latest commit: b1e80749e70586de9f78d82d467a71b29afe24d8
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
- :white_check_mark: login: rxliuli / name: rxliuli (39bc87ef30c2c984acc826789c6a6693060e8dee, d6a0b90c8ac129f58945120b0f5d14f0e6d2929c, c3e9ec0044f305d22548a62762416ef6dfbfb018, b1e80749e70586de9f78d82d467a71b29afe24d8)
- :x: The commit (7d78754f9b6fe3b1b91bf061c6d18a66fc1783c2, 912c516e086e8de7964a0272476f88cec1b34ed3, ae6291387a59f452b4b6098192fa8588d4c3fa35, 04113a2f747c13b6fe3ded76b13bbd486b43bc54, 51fc85c4e48685fafe21969283baf88ae714c531, 1e3f102fcd81254a699ed9accde2bab5acb810ac, 186a5a1ac4bcc25ac131fd814daf825ef3b3543d). This user is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.
I have signed up for EasyCLA, what else do I need to do to trigger the next process? @acao
@rxliuli ah, the email used for CLA has to be the same email that’s used in each commit. Also I have to approve the action workflows for the first PR from a user because we have had malicious PRs
It looks like we need to use a base image that already has pnpm ideally, or we can use an action to manually install pnpm for each workflow
It looks like we need to use a base image that already has pnpm ideally, or we can use an action to manually install pnpm for each workflow
Just run npm i -g pnpm
Do you want to change the command to install dependencies on netlify? I have observed that the dependencies are still installed using npm and then fail
https://app.netlify.com/sites/graphiql-test/deploys/625f4fd9e4fab7000994463a
@rxliuli awesome! Also, weird, not sure why i had to re-approve the actions
Build & Test I know what's wrong, it's fixed. But why does lint fail, it seems strange, do I need to fix these lint and ts errors? Or some configuration was accidentally changed. . .
https://github.com/graphql/graphiql/runs/6087316492?check_suite_focus=true
@rxliuli something is off with lint staged, so for now you need to run pnpm format.
I can’t remember why i migrated the actions to re-install modules from the shared cache individually rather than having a shared action for npm install and build. It started with a seperate install => cypress workflow that seemed to complete 1 minute faster somehow. Either way this is great because it will save us 30-50s per action based on your experiments
Hmm, the typescript version must be resolving differently, either way you can use the ts-expect-error comment if you want since they are mostly just handling exceptions
I restored the version of the dependencies from the previous yarn.lock, including typescript, sorry, I didn't notice the change of the dependency version before
webpack4 is not compatible with pnpm, it's a bit tricky, have you guys done webpack4 => webpack5? Or we can use rollup? or use faster esbuild? https://github.com/webpack/webpack/issues/5087
I think @acao was thinking of Vite, which I'd be totally ok with. From my side you can go ahead and move to Vite. Curious what @acao thinks
This was the blocker we had run into before with pnpm, thus why i raised the issue of webpack support early on! Our plan was to migrate to vite instead, and drop babel and webpack altogether. But we may have to upgrade to webpack 5 as a transitional step. It will have to wait as I’m sick with covid and my co maintainers are focused on other things at the moment.
when we originally upgraded to webpack from browserify (yes when I inherited the project it still used browserify), webpack 4 was beta and webpack 5 did not exist. So many generations of tooling 😭
With vite the goal was to get rid of the duplicate typescript project references tree and use vite to bundle each library for it‘s relevant exports, but I’m having trouble finding something that can re-create tsc —build —watch on a monorepo scale. @nx
might have something for this, I can’t seem to find any vite and/or rollup plugins that solve this problem. Turnorepo purports to solve this problem but is only available commercially
This was the blocker we had run into before with pnpm, thus why i raised the issue of webpack support early on! Our plan was to migrate to vite instead, and drop babel and webpack altogether. But we may have to upgrade to webpack 5 as a transitional step. It will have to wait as I’m sick with covid and my co maintainers are focused on other things at the moment
Is there any suggestion, currently only one **Test PR / Cypress E2E Suite (pull_request) ** can not pass, I don't use webpack very much so don't know much about the configuration here packages/graphiql/resources/webpack.config.js
With vite the goal was to get rid of the duplicate typescript project references tree and use vite to bundle each library for it‘s relevant exports, but I’m having trouble finding something that can re-create tsc —build —watch on a monorepo scale.
@nx
might have something for this, I can’t seem to find any vite and/or rollup plugins that solve this problem
In production, we wrote a plugin to handle this, pointing the module to src/index.ts so that we don't have to rebuild dist/index.js every time the lib is modified
ref: https://github.com/rxliuli/liuli-tools/blob/master/libs/rollup-plugin-ts-alias/README.md
Like I mentioned in #2185, you will need to find a compatibility mode or workaround for webpack 4, jest, cypress, etc, which has been what kept us from switching to pnpm im previous attempts. Currently the deploy preview and build users import from cdn isn’t building.
Otherwise a webpack 5 upgrade will take some sizeable effort and will definitely need to come in a separate PR, since it heavily impacts the actual bundle we ship to the CDNs, which is used more than even the esm module!
@rxliuli that plugin looks very promising thanks! Apologies there seems to be a delay with my LTE connection, each of your comments is only visible once I make a comment? So I’m just seeing your reply now
I think we can do this best in two or more PRs:
- swap out webpack for vite - we don’t need to worry about bundling every library yet. Leave duplicate project references for now. Merge.
- then, rebase this PR for pnpm with that upstream change
- Then, worry about replacing the duplicate references for a modern bundling and even
exports
strategy in an additional PR
Thanks for providing this context @acao ! That plan makes total sense to me. @rxliuli do you have capacity to work on the steps Rikki outlined?
Thanks for providing this context @acao ! That plan makes total sense to me. @rxliuli do you have capacity to work on the steps Rikki outlined?
I will try it tomorrow
closing for #3054