Skip to content

fix(bazel/rules_angular): update worker to use user-configured TypeScript instead of npm version#3597

Merged
alan-agius4 merged 1 commit intoangular:mainfrom
alan-agius4:worker-ts-version
Apr 9, 2026
Merged

fix(bazel/rules_angular): update worker to use user-configured TypeScript instead of npm version#3597
alan-agius4 merged 1 commit intoangular:mainfrom
alan-agius4:worker-ts-version

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

Ensure that the users typescript version is used.

…ript instead of npm version

Ensure that the users typescript version is used.
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Apr 9, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the bazel/rules/rules_angular/src/worker/BUILD.bazel file to ensure that a user-configured TypeScript version is used for compilation instead of the default npm version. The review feedback suggests making the inline comment for this change more concise for better readability and consistency with other comments in the file.

# output, but it's still necessary for some foundational utils like virtual FS.
"//:node_modules/@angular/compiler-cli", # compiler from npm.
"//:node_modules/typescript", # typescript from npm.
":node_modules/typescript", # user-configured typescript. Ensure that the users TypeScript version is always used for the compilation.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this was previously using the version from npm because we can cause mismatches between the expected typescript types for the local envinronment (the users TS version) and the version that @angular/compiler-cli expects from npm.

Is that not a concern? Or I am misunderstanding?

Copy link
Copy Markdown
Contributor Author

@alan-agius4 alan-agius4 Apr 9, 2026

Choose a reason for hiding this comment

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

The problem with that approach is that any usage of ts_project will use the built-in version of the TypeScript instead of what is supplied by the users.

In the case of having mismatches, I think we need to come up with a better solution or use an @angular/compiler-cli version that supports the TypeScript versions that are used in both main and patch.

One other alternative would be not using the the @angular/compiler-cli FS for plain TS compilations but that is a larger task.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, If we are okay with moving the "error point" to the angular compiler-cli version like this, thats fine with me.

Copy link
Copy Markdown
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

@alan-agius4 alan-agius4 merged commit 320dbd1 into angular:main Apr 9, 2026
13 checks passed
@alan-agius4
Copy link
Copy Markdown
Contributor Author

This PR was merged into the repository. The changes were merged into the following branches:

@alan-agius4 alan-agius4 deleted the worker-ts-version branch April 9, 2026 15:54
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants