build: Use NPM Workspace instead of Lerna to bootstrap #1142

darryltec · 2023-03-20T23:00:45Z

Motivations

Lerna is planning to deprecate bootstrap on their V7 since NPM can do it in a much better way. With the Node 18 upgrade, the need to move has become more apparent since it totally broke the way it worked for us.

Known issues

It'll throw a dependency warning and that's related to storybook upgrade. That comes later.

Changes

Added

Changed

Switch Lerna magic to NPM Workspace
- In doing so, we can delete the packages package-lock.json since they're managed on the root now. No more confusing package-lock changes
Had to work around NPM's lack of topological order when bootstrapping
- Basically, npm would run prepare on each packages in alphabetical order but components need design to be built first.
- This works around that limitation by firing the scripts manually through npm run [script] -w which respects the order under workspaces
- [RRFC] Add flag for running NPM commands in transitive dependencies npm/rfcs#548

Deprecated

Removed

Fixed

Security

Testing

Run nvm use
Run npm ci successfully with no error. Warning is fine.
Run npm start and storybook runs locally

In Atlantis we use Github's built in pull request reviews.

JOB-63633

Basically, npm would run `prepare` on each packages in alphabetical order but components need design to be built first. This works around that limitation by firing the scripts manually through `npm run [script] -w` which respects the order under `workspaces` npm/rfcs#548

cloudflare-workers-and-pages · 2023-03-21T12:54:22Z

Deploying with Cloudflare Pages

Latest commit:	`4947ff9`
Status:	🚫 Build failed.

View logs

@rodrigoeidelvein

Thanks @rodrigoeidelvein for the workaround!

rodrigoeidelvein · 2023-03-22T14:44:27Z

~~@darryltec running npm ci is throwing the following error for me:~~
~~npm ERR! Cannot read property '@std-proposal/temporal' of undefined~~

~~I deleted all node_modules and I still got the error~~

nvm, just had to run nvm use, thanks @darryltec

rodrigoeidelvein · 2023-03-22T19:02:34Z

I think Cloudfare failed because the build command is set to npm run build-storybook instead of storybook:build, we may have to ask @kingstonfung to change it once we land this PR.

darryltec · 2023-03-22T19:08:53Z

@rodrigoeidelvein cloudflare failed becauase it's building on node 18 which it can't haha. Cloudflare fires npm run build

rodrigoeidelvein

I'm going to make a list of things that I noticed here, just to see if I understood correctly:

Removed the sum_lockfiles because there's no package-lock.json in packages/* anymore? Are they all in the root package-lock?
--no-optional == -omit=optional?
You reorganized the npm command in the root package.json, and renamed some of them, but the only thing that changed inside the commands is the export NODE_OPTIONS=--openssl-legacy-provider in Storybook?
Why did some snapshots change?
Loved the bonus refactor on FormatRelativeDateTime tests, was it failing?

darryltec · 2023-03-22T19:23:30Z

Removed the sum_lockfiles because there's no package-lock.json in packages/* anymore? Are they all in the root package-lock?

Yup

--no-optional == -omit=optional?

Yup

You reorganized the npm command in the root package.json, and renamed some of them, but the only thing that changed inside the commands is the export NODE_OPTIONS=--openssl-legacy-provider in Storybook?

Sorted alphabetically. Yes and also this

atlantis/package.json

Line 31 in 3d589ee

"postinstall": "npm run bootstrap --workspaces --if-present",

Why did some snapshots change?

Normally happens on node update

Loved the bonus refactor on FormatRelativeDateTime tests, was it failing?

.forEach on tests removes the ability for the test to run cleanup automatically. Which broke the test. Writing it properly fixes it. And yeah, totally broke and threw weird errors.

packages/components/package.json

rodrigoeidelvein

Niiiice! Looks good to me!

npm ci works
storybook runs and builds

I just would like an approval from @MichaelParadis too

MichaelParadis

LGTM!

@rodrigoeidelvein

* build: update nvmrc to use 18 * build: repackage package locks * build: temporarily set legacy-peer-deps to true * build: Use NPM Workspace instead of Lerna to bootstrap (#1142) * build: use NPM workspace instead of Lerna * fix: workaround npm's lack of topological oder Basically, npm would run `prepare` on each packages in alphabetical order but components need design to be built first. This works around that limitation by firing the scripts manually through `npm run [script] -w` which respects the order under `workspaces` npm/rfcs#548 * fix: reorder scripts for my own sanity * build: update circle to not look for lock files * build: use node 18 image on circle ci * fix: js and css linters * build: use -omit=optional * fix: update time test work on node 18 * fix: don't update snaps * fix: update some snaps * build: make storybook run on node 18 Thanks @rodrigoeidelvein for the workaround! * fix: reinstall ts-node * chore: use lerna recommended setting for symlink * fix: refix storybook by using js file * fix: match * on preventManualRelease check * docs: Node 18 update readme (#1149) * Update versions in readme * Update engines in package.json * fix: reset package-lock * chore: strict engine and package lock * docs: Fix cloudflare Pages deploy (#1150) * Trying build * Correcting variables * Removed resource class Co-authored-by: Darryl Tec <darryl.t@getjobber.com> --------- Co-authored-by: Kingston Fung <kingstonfung@gmail.com> Co-authored-by: Darryl Tec <darryl.t@getjobber.com> * fix: update package-lock.json --------- Co-authored-by: Michael Paradis <michael.p@getjobber.com> Co-authored-by: Kingston Fung <kingstonfung@gmail.com>

Darryl Tec added 3 commits March 20, 2023 16:23

build: use NPM workspace instead of Lerna

2c56ad6

fix: reorder scripts for my own sanity

bcca264

build: update circle to not look for lock files

ef2dba8

darryltec force-pushed the use-npm-workspace-instead-of-lerna branch from ea33080 to ef2dba8 Compare March 21, 2023 12:55

build: use node 18 image on circle ci

cf32562

darryltec force-pushed the use-npm-workspace-instead-of-lerna branch from a595a3a to cf32562 Compare March 21, 2023 13:04

Darryl Tec added 7 commits March 21, 2023 07:58

fix: js and css linters

26926f9

build: use -omit=optional

26a679e

fix: update time test work on node 18

7fd1bf4

fix: don't update snaps

2d8a386

fix: update some snaps

740d4dc

build: make storybook run on node 18

f4d52f1

Thanks @rodrigoeidelvein for the workaround!

fix: reinstall ts-node

3d589ee

darryltec requested a review from a team March 21, 2023 17:52

jobbiebot bot added review requested Team Atlantis labels Mar 21, 2023

MichaelParadis self-requested a review March 22, 2023 19:07

rodrigoeidelvein reviewed Mar 22, 2023

View reviewed changes

MichaelParadis reviewed Mar 22, 2023

View reviewed changes

packages/components/package.json Show resolved Hide resolved

Darryl Tec added 3 commits March 24, 2023 06:27

chore: use lerna recommended setting for symlink

f7fed28

fix: refix storybook by using js file

17f19b0

fix: match * on preventManualRelease check

4947ff9

rodrigoeidelvein approved these changes Mar 24, 2023

View reviewed changes

jobbiebot bot removed the review requested label Mar 24, 2023

jobbiebot bot added the approved label Mar 24, 2023

MichaelParadis approved these changes Mar 24, 2023

View reviewed changes

darryltec merged commit 76337c4 into upgrade-to-node-18 Mar 24, 2023

darryltec deleted the use-npm-workspace-instead-of-lerna branch March 24, 2023 14:25

darryltec mentioned this pull request Mar 24, 2023

feat: Upgrade to Node 18 [Feature Branch][JOB-62820] #1141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Use NPM Workspace instead of Lerna to bootstrap #1142

build: Use NPM Workspace instead of Lerna to bootstrap #1142

darryltec commented Mar 20, 2023 •

edited

Loading

cloudflare-workers-and-pages bot commented Mar 21, 2023 •

edited

Loading

rodrigoeidelvein commented Mar 22, 2023 •

edited

Loading

rodrigoeidelvein commented Mar 22, 2023

darryltec commented Mar 22, 2023

rodrigoeidelvein left a comment •

edited

Loading

darryltec commented Mar 22, 2023

rodrigoeidelvein left a comment

MichaelParadis left a comment

build: Use NPM Workspace instead of Lerna to bootstrap #1142

build: Use NPM Workspace instead of Lerna to bootstrap #1142

Conversation

darryltec commented Mar 20, 2023 • edited Loading

Motivations

Known issues

Changes

Added

Changed

Deprecated

Removed

Fixed

Security

Testing

cloudflare-workers-and-pages bot commented Mar 21, 2023 • edited Loading

Deploying with Cloudflare Pages

rodrigoeidelvein commented Mar 22, 2023 • edited Loading

rodrigoeidelvein commented Mar 22, 2023

darryltec commented Mar 22, 2023

rodrigoeidelvein left a comment • edited Loading

Choose a reason for hiding this comment

darryltec commented Mar 22, 2023

rodrigoeidelvein left a comment

Choose a reason for hiding this comment

MichaelParadis left a comment

Choose a reason for hiding this comment

darryltec commented Mar 20, 2023 •

edited

Loading

cloudflare-workers-and-pages bot commented Mar 21, 2023 •

edited

Loading

rodrigoeidelvein commented Mar 22, 2023 •

edited

Loading

rodrigoeidelvein left a comment •

edited

Loading