chore: Move to yarn 4 and workspaces#10321
Conversation
chrisgervang
left a comment
There was a problem hiding this comment.
I'm not a fan of how fragile yarn tends to be - workspaces and compatibility issues with PnP can be frustrating to users. Also, it's currently easy to copy/paste an example into your own project to adopt deck, or codesandbox to reproduce a bug. I don't want to loss those adoption channels with the added complexity
What if we used npm 7? It has many of the same benefits as yarn, it's actively maintained (yarn v1 is quite old), and is very easy for users adopting deck to work with
| // pad token with zeros to make the length 16 | ||
| const paddedToken = token.padEnd(16, '0'); | ||
| return Long.fromString(paddedToken, 16); | ||
| return Long.fromString(paddedToken, 16).toString(); |
There was a problem hiding this comment.
Let's split this out
There was a problem hiding this comment.
I understand that having seemingly unrelated changes in a PR is annoying but this is related to typescript version being bumped as part of satisfying all the package.json.
To split this out I would need to make a separate PR to bump typescript and fix issue or downgrade typescript in other places.
There was a problem hiding this comment.
I just want to make sure we're thinking through the implications and covering bug fixes with tests. I'm only focusing on testing the build system in this PR
|
|
||
| ### Usage | ||
|
|
||
| Copy the content of this folder to your project. |
There was a problem hiding this comment.
The intent @Pessimistress shared with me years ago was that example folders are meant to be standalone so that they're easy to bring into you own projects.
Workspaces adds a layer of complexity for adopters, and I'm not convinced its the right direction to go
There was a problem hiding this comment.
We definitely don't have to use workspace version specifiers in the examples I get that might be a separate discussion point.
That said, the version specifiers in the examples are often out of date which doesn't exactly help user, and during alpha/beta release cycles it gets worse.
FWIW, this branch is not using PnP. Both root and website are configured with nodeLinker: node-modules, so Yarn writes a normal node_modules install. I’m removing the .pnp.* ignore entry. I personally consider workspace installs a real "luxury", have personally not had issues with it in our other repos. Just fast reliable installs across the repo and CI.
Agreed, I am removing the "workspace" version specifiers, that was a step too far.
I have no experience with recent npm but understand that it has improved a lot. Most projects I am familiar with seem to be going pnpm these days though. I am not aware of anything we would gain by switching that is meaningully better than yarn 4 + workspaces, but if someone thinks it is worth it and wants to put in the work, I am not against it. We do use yarn 4 + workspace in our other repos so that does make the experience a little more consistent for a multi-repo developer like myself. |
|
@chrisgervang Appreciate the comments! Addressed the bigger issues, and cleaned up the PR. File churn now down from 158 to 36 files! |
chrisgervang
left a comment
There was a problem hiding this comment.
Thanks for addressing my feedback. I think it's important to keep the adoption simple, so I appreciate not using the workspace version specifiers.
Approving to unblock but I would prefer to split out the bug fix found by a typescript upgrade so its documented well and covered, and split out module beta upgrades
| "scripts": { | ||
| "watch": "npm run build -- --watch", | ||
| "build": "ocular-bundle ./src/index.js --output=./dist/index.js --globalName= --sourcemap --format=umd --externals=@jupyter-widgets/base", | ||
| "watch": "npm run build-bundle -- --watch", | ||
| "build": "npm run build-bundle", | ||
| "build-bundle": "ocular-bundle ./src/index.js --output=./dist/index.js --globalName= --sourcemap --format=umd --externals=@jupyter-widgets/base", |
There was a problem hiding this comment.
What does this change?
| // pad token with zeros to make the length 16 | ||
| const paddedToken = token.padEnd(16, '0'); | ||
| return Long.fromString(paddedToken, 16); | ||
| return Long.fromString(paddedToken, 16).toString(); |
There was a problem hiding this comment.
I just want to make sure we're thinking through the implications and covering bug fixes with tests. I'm only focusing on testing the build system in this PR
| uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: 'yarn' |
There was a problem hiding this comment.
I'm looking into if this increases build times
|
Could you take a pass on the PR description before merging for anything that is out of date? |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I've asked the LLM to review installation times and this is what it found:
Root cause: The Options to fix:
The regression adds ~2-3 minutes of extra wall-clock time per CI run. |
Resolved yarn.lock conflicts by regenerating lock files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 258145b. Configure here.
| uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: 'yarn' |
There was a problem hiding this comment.
CI cache removed without replacement, causing slower builds
Medium Severity
The cache: 'yarn' option was removed from actions/setup-node across all CI workflows (test-node, test-website, test-python, release, website deploy) without adding any replacement caching mechanism such as actions/cache. PR discussion confirms this causes ~2.5x slower dependency install times on every CI run. While Yarn 4 may need different cache path configuration, the caching capability can be restored using actions/cache with the correct Yarn 4 global cache directory.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 258145b. Configure here.


Summary
Migrates deck.gl to Yarn 4.15 workspaces with
nodeLinker: node-modules, and installs all package-managed examples from the repo root.Motivations
Changes
packageManagertoyarn@4.15.0..yarnrc.ymlwith:enableScripts: truenodeLinker: node-modulesmodules/*to:modules/*examples/**deck.gl/@deck.gl/*example deps toworkspace:*.modules/*.yarn install.yarn.lockwith Yarn 4.15.@deck.gl/*peer ranges from~9.3.0-alpha.1to~9.3.0-beta.2.Verification
corepack yarn --version->4.15.0corepack yarn install --mode=skip-buildcorepack yarn install --immutable --mode=skip-buildcorepack yarn workspaces list --jsoncorepack yarn test-fastdeckgl-example-react-basicdeckgl-example-pure-js-basicdeckgl-examples-scatterplotdeckgl-examples-galleryNotes
Plain
yarn installruns the existing rootpostinstallscript (npx playwright install chromium). Lockfile verification used--mode=skip-buildto avoid browser-install side effects during validation.Note
Medium Risk
Touches all install/CI/release paths and dependency resolution across modules and many examples; incorrect workspace or resolution pins could break builds or publishing, though runtime library changes are minimal (one S2 helper return type).
Overview
Upgrades the monorepo from Yarn 1 to Yarn 4.15 with
node-moduleslinking and a rootpackageManagerpin, adding.yarnrc.yml(scripts enabled, peer-warning filters) and ignoring.yarn/in git.Workspaces now include
examples/**alongsidemodules/*, so examples install from one root lockfile;bootstrapno longer runsyarnitself, and CI/release/website flows usecorepack enable,yarn install --immutable, thenyarn bootstrap(droppingsetup-node’s yarn cache). The website keeps its own Yarn 4 config and uses immutable installs intest-website/ deploy steps; gallery build scripts rely on the root install instead of per-folderyarn.Root
resolutionspin all@deck.gl/*/deck.glto9.3.0-beta.2; examplepackage.jsonfiles get small metadata/order fixes.getIdFromTokenins2-utilsreturns a string id (viaLong.toString()) instead of aLongobject. Minor tooling tweaks: Docusaurus source-map rule excludesnode_modules, jupyter-widget build script split, and a mapbox default-import eslint comment in a get-started example.Reviewed by Cursor Bugbot for commit 258145b. Bugbot is set up for automated code reviews on this repo. Configure here.