Skip to content

Conversation

@devversion
Copy link
Member

Enables bazel managed node modules symlinking (similar to what framework does)

  • Easier way to make manual changes to debug issues
  • Less disk size since we do not need to maintain two instances of node_modules
  • Caching of node modules in CI. Bazel previously discarded the whole node
    modules folder if the lock file changed.

Note: The postinstall patches script had to be updated to cover the following cases:

  • It should throw if a new patch is added but does not apply
  • It should not throw if the same patch is applied on restored node_modules (i.e. from cache)
  • CircleCI should not use cached node modules if the postinstall script changes. This previously wasn't necessary since Bazel always invalidated the whole node modules folder.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 12, 2019
Enables bazel managed node modules symlinking (similar to
what framework does). We benefit from:

* Easier way to make manual changes to debug issues
* Less disk size since we do not need to maintain two instances of
`node_modules`
* Caching of node modules in CI. Bazel previously discarded the
whole node modules folder if the lock file changed.
@devversion devversion force-pushed the build/enable-node-modules-symlinking branch from 1f8f19f to 02a8a9f Compare November 12, 2019 14:19
@devversion devversion marked this pull request as ready for review November 12, 2019 14:31
@devversion devversion requested review from a team and jelbourn as code owners November 12, 2019 14:31
@devversion devversion added pr: merge safe target: patch This PR is targeted for the next patch release labels Nov 12, 2019
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 12, 2019
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn merged commit 022acf8 into angular:master Nov 20, 2019
jelbourn added a commit to jelbourn/components that referenced this pull request Nov 21, 2019
This reverts commit 022acf8.

Reverting this fixed the npm package build being broken.
mmalerba pushed a commit that referenced this pull request Nov 21, 2019
This reverts commit 022acf8.

Reverting this fixed the npm package build being broken.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants