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.

Sign up for GitHub

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

Jump to bottom

adev: Local packages not linked properly for development #54858

Open
devversion opened this issue Mar 14, 2024 · 3 comments
Open

adev: Local packages not linked properly for development #54858

devversion opened this issue Mar 14, 2024 · 3 comments
Assignees
@josephperrott
Labels
area: adev Angular.dev documentation area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone
Backlog

Comments

@devversion
Copy link
Member

Unlike with aio, adev only links top-level @angular/ packages to their npm archives from HEAD. Transitive dependencies are not properly updated, like we do for AIO— this means that adev may run with older versions of the Angular compiler because e.g. the CLI devkit would have their own transitive dependency.

This can cause subtle build issues when making changes in the compiler. We should investigate if this is still needed, but not throw away the effort put into AIO:

angular/aio/local_packages_util.bzl

Lines 15 to 25 in 700c052

def link_local_packages(all_aio_deps):
"""Create targets needed for building AIO against local angular packages.
Creates targets that link Angular packages, as well as targets to be used
in place of any deps required to build and test AIO. These targets filter
out any transitive deps on the npm packages and must be used in place of
any original list of deps.
Use the helper `substitute_local_package_deps()` to translate a list of deps
to the equivalent "filtered" target that this rule creates.

Additional context: 6cc3256
@devversion devversion added the area: adev Angular.dev documentation label Mar 14, 2024
@ngbot ngbot bot added this to the needsTriage milestone Mar 14, 2024
@bencodezen bencodezen added the P2 The issue is important to a large percentage of users, with a workaround label Mar 14, 2024
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Mar 14, 2024
@bencodezen bencodezen added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed P2 The issue is important to a large percentage of users, with a workaround labels Mar 14, 2024
@bencodezen bencodezen added the area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) label Mar 27, 2024
@JeanMeche
Copy link
Member

This is probably related, I've noticed that the ADEV app is not build against the local packages. See #55161 where I had to rely on a temporary patch-package to fix an issue which was already fixed locally.

@devversion
Copy link
Member Author

Yeah, the packages are not picked up and this makes it difficult to also use new compiler features in adev that aren't released yet

@devversion
Copy link
Member Author

I believe this would also fix issues like:

angular-dev:serve                        ................................................................................ (0/1) 
This version of CLI is only compatible with Angular versions ^17.0.0 || ^17.3.0-next.0,
but Angular version 18.0.0-next.3 was found instead.
Please visit the link below to find instructions on how to update Angular.

where the angular-devkit accidentally sees @angular/core from the @angular/docs package installed in then workspace-level dependencies.

devversion added a commit to devversion/angular that referenced this issue Apr 10, 2024
@devversion
build(docs-infra): fix version mismatch of local built packages
37d945b 
There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. angular#54858 (comment)
- Compiler changes not being picked up. angular#54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.
@devversion devversion mentioned this issue Apr 10, 2024
Closed
devversion added a commit to devversion/angular that referenced this issue Apr 10, 2024
@devversion
build(docs-infra): fix version mismatch of local built packages
efd8235 
There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. angular#54858 (comment)
- Compiler changes not being picked up. angular#54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.
devversion added a commit to devversion/angular that referenced this issue Apr 10, 2024
@devversion
build(docs-infra): fix version mismatch of local built packages
bed8ff9 
There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. angular#54858 (comment)
- Compiler changes not being picked up. angular#54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.
atscott pushed a commit that referenced this issue Apr 10, 2024
@devversion @atscott
build(docs-infra): fix version mismatch of local built packages (#55282)
f828984 
There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. #54858 (comment)
- Compiler changes not being picked up. #54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.

PR Close #55282
atscott pushed a commit that referenced this issue Apr 10, 2024
@devversion @atscott
build(docs-infra): fix version mismatch of local built packages (#55282)
7676d8f 
There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. #54858 (comment)
- Compiler changes not being picked up. #54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.

PR Close #55282
iteriani pushed a commit to iteriani/angular that referenced this issue Apr 11, 2024
@devversion @iteriani
build(docs-infra): fix version mismatch of local built packages (angu…
8a347ec 
…lar#55282)

There is quite some trickery going on with the adev build related to
local packages:

- Adev builds using npm packages from `/node_modules`
- At runtime, we are adding `HEAD` packages for e.g. `@angular/core` to
  the bundles.
- At build time, the CLI, or Angular devkit may accidentally resolve to
  `@angular/core` from `/node_modules/`— which is the core version from
  npm, transitively installed via `@angular/docs`.

This causes a version mismatch, leading to issues like:

- CLI throwing because of a mismatch. angular#54858 (comment)
- Compiler changes not being picked up. angular#54858 (comment)

This commit attempts to fix this by:

- Linking all Angular `HEAD` packages into `adev/node_modules`. The
  current logic attempts to link into `/node_modules`, but this does not
  override existing `@angular/core`!
- Linking all direct external NPM packages, like
  `@angular_devkit/build-angular` into `adev/node_modules` without their
  transitive deps. This allows proper resolution of e.g. compiler as
  node looks in `adev/node_modules` first, and falls back for the rest
  to the execroot `node_modules`, or symlink target destination (if
  `preserveSymlinks=false`).

Note: This is still not 100% ideal because a direct external NPM
dependency may have a transitive dependency that has another transitive
dependency on `@angular/core`. In those cases, the may be a conflict
that is not resolvable until we switch to a Bazel toolchain with better
first party resolution support.

PR Close angular#55282
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@josephperrott josephperrott

Labels
area: adev Angular.dev documentation area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@JeanMeche @bencodezen @devversion @josephperrott