Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Update all the package-locks #264

Merged
merged 13 commits into from
Jan 8, 2019
Merged

Update all the package-locks #264

merged 13 commits into from
Jan 8, 2019

Conversation

bcmn
Copy link
Contributor

@bcmn bcmn commented Jan 7, 2019

Resolves #263

Fixing the out of sync package-lock.json on latest was made more difficult by the psammead-test-helpers issue -- Many of the nested packages had package-lock.json files which seemed to be pointing to 0.1.3, a version not present on NPM.

Following this issue, when pulling in latest & wanting to resolve package-lock differences, run npm run install:packages. There is an equivalent package-lock-only command: npm run ci:packages.

  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@bcmn bcmn self-assigned this Jan 7, 2019
radiocontrolled
radiocontrolled previously approved these changes Jan 8, 2019
dr3
dr3 previously approved these changes Jan 8, 2019
@bcmn bcmn dismissed stale reviews from dr3 and radiocontrolled via 920526e January 8, 2019 11:23
@bcmn
Copy link
Contributor Author

bcmn commented Jan 8, 2019

So... Working with @radiocontrolled, we observed that when the previous postinstall step was configured, npm i would fail after pulling this branch into another branch.

However, removing the postinstall script & running npm i && lerna bootstrap --ci --hoist --force-local (the content of the postinstall script) would be successful.

As such, we have removed the postinstall script & added npm i && lerna bootstrap --ci --hoist --force-local as ci:packages.

sareh
sareh previously approved these changes Jan 8, 2019
Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Great investigation work!

Copy link
Contributor

@radiocontrolled radiocontrolled left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re:

Following this issue, when pulling in latest & wanting to resolve package-lock differences, run npm run install:packages. There is an equivalent package-lock-only command: npm run ci:packages.

Could we document this in the readme as it will be a recurring 'gotcha'?

@pjlee11
Copy link
Contributor

pjlee11 commented Jan 8, 2019

This looks to be working having removed the postinstall npm script and instead using npm run install:packages. Lerna bootstrap appears to hoist the nanoid dependency to the top level package.json, before then pruning nanoid out of the package.json. This results in the top level of psammead having node_modules/ containing nanoid and the site-wide-links component which had nanoid in its package.json to not require nanoid in its own node_modules/.

A screenshot of the node_modules described above
image

Copy link
Contributor

@pjlee11 pjlee11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 👍

Copy link
Contributor

@radiocontrolled radiocontrolled left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job 🎆

@jamesbrumpton
Copy link
Contributor

LGTM 👍

@bcmn bcmn merged commit 871fb17 into latest Jan 8, 2019
@bcmn bcmn deleted the fix-package-locks branch January 8, 2019 14:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hoisting failure on latest
6 participants