Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further clean-up rules_nodejs npm workspace and remove yarn.lock #29779

Merged
merged 7 commits into from
Mar 11, 2025

Conversation

devversion
Copy link
Member

See individual commits

@angular-robot angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label Mar 7, 2025
@alan-agius4
Copy link
Collaborator

Note: we need to update the renovate dev-infra action to install PNPM.

@devversion devversion force-pushed the wip-2 branch 3 times, most recently from 27b5cbb to 0ba3f65 Compare March 10, 2025 16:25
@devversion
Copy link
Member Author

Note: we need to update the renovate dev-infra action to install PNPM.

I think @josephperrott took care of this already.

@josephperrott
Copy link
Member

Yes, the renovate action should not have pnpm installed in its environment as of last thursday.

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.

.yarn/install-state.gz Should not be installed

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 after the requested changes are addressed.

Copy link
Member

@clydin clydin left a comment

Choose a reason for hiding this comment

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

As mentioned, .yarn/install-state.gz should be removed. Also, patch-package should be removed as well since it's no longer needed.
Otherwise, LGTM.

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.
We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.
`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.
This will allow us to use pnpm.
Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.
The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
@devversion devversion requested a review from alan-agius4 March 11, 2025 07:23
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Mar 11, 2025
@devversion devversion marked this pull request as ready for review March 11, 2025 07:23
@devversion
Copy link
Member Author

Addressed feedback. Also note: I will improve/clean-up more of the documentation in a follow-up!

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 11, 2025
@devversion devversion merged commit 9dd3f03 into angular:main Mar 11, 2025
33 of 34 checks passed
@devversion devversion deleted the wip-2 branch March 11, 2025 09:05
devversion added a commit to devversion/angular-cli that referenced this pull request Mar 11, 2025
angular#29779)

* build: disconnect `@npm` workspace from main project

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.

* build: drop `_rjs` suffix from `ts_project` targets

We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.

* build: remove yarn

* build: avoid duplicated dependencies at top-level

`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.

* build: update checkout github action

This will allow us to use pnpm.

* build: update node to avoid strict-engines error caused by `npm`

Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.

* build: fix missing dependency for spec bundling

The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
devversion added a commit to devversion/angular-cli that referenced this pull request Mar 11, 2025
angular#29779)

* build: disconnect `@npm` workspace from main project

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.

* build: drop `_rjs` suffix from `ts_project` targets

We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.

* build: remove yarn

* build: avoid duplicated dependencies at top-level

`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.

* build: update checkout github action

This will allow us to use pnpm.

* build: update node to avoid strict-engines error caused by `npm`

Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.

* build: fix missing dependency for spec bundling

The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
devversion added a commit to devversion/angular-cli that referenced this pull request Mar 11, 2025
angular#29779)

* build: disconnect `@npm` workspace from main project

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.

* build: drop `_rjs` suffix from `ts_project` targets

We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.

* build: remove yarn

* build: avoid duplicated dependencies at top-level

`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.

* build: update checkout github action

This will allow us to use pnpm.

* build: update node to avoid strict-engines error caused by `npm`

Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.

* build: fix missing dependency for spec bundling

The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
devversion added a commit to devversion/angular-cli that referenced this pull request Mar 11, 2025
angular#29779)

* build: disconnect `@npm` workspace from main project

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.

* build: drop `_rjs` suffix from `ts_project` targets

We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.

* build: remove yarn

* build: avoid duplicated dependencies at top-level

`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.

* build: update checkout github action

This will allow us to use pnpm.

* build: update node to avoid strict-engines error caused by `npm`

Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.

* build: fix missing dependency for spec bundling

The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
devversion added a commit that referenced this pull request Mar 11, 2025
#29779)

* build: disconnect `@npm` workspace from main project

This will speed up significantly as we don't need to fetch all
dependencies again just for the `@npm` repository that is at this point
only leveraged by the `ng_package` rule for some of its dependencies.

This commit allows us to drop the `yarn.lock` and Aspect lock files, and
allows us to independently migrate `ng_package` to `rules_js`.

It also allows us to drop the `_rjs` TS interop layer in follow-up commits.

* build: drop `_rjs` suffix from `ts_project` targets

We don't need the `ts_project` interop in principle
at this point. We only have one remaining instance left for the SSR
`ng_package` integration. This commit cleans up all usages.

* build: remove yarn

* build: avoid duplicated dependencies at top-level

`rules_js` seems to be sensitive if there are similar versions of the same
package installed, but with differently matched peer dependencies. This
is fine because we can (and should long-term) move those dependencies to
their package-local `package.json` files. This commit unblocks the
migration and highlights how we can move deps to the individual packages
in the future.

* build: update checkout github action

This will allow us to use pnpm.

* build: update node to avoid strict-engines error caused by `npm`

Avoids:

```
Lockfile is up to date, resolution step is skipped
 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "npm@11.2.0".

Expected version: ^20.17.0 || >=22.9.0
Got: v20.11.1
```

Note that we won't update the WORKSPACE test version as that would mean
we need to update the Node engines for shipped packages; and we can't do
this right now without introducing a breaking change.

* build: fix missing dependency for spec bundling

The beasties JS sources weren't available for bundling in the
`bazel-bin`, and this surfaced in RBE. This commit fixes this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants