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

fix(compiler-cli): detect when the linker is working in unpublished angular and widen supported versions #54439

Closed
wants to merge 1 commit into from

Conversation

josephperrott
Copy link
Member

When the linker is running using an unpublished version of angular, locally built, the version will be 0.0.0. When encountering this situation, the range that for the linker map support is considered to be *.*.* allowing for the linker to work at build time with packages built with versioned angular.

Most notably this allows for us to properly use the linker in building our documentation site with the locally built version of angular.

…ngular and widen supported versions

When the linker is running using an unpublished version of angular, locally built, the version will be `0.0.0`.
When encountering this situation, the range that for the linker map support is considered to be `*.*.*` allowing
for the linker to work at build time with packages built with versioned angular.

Most notably this allows for us to properly use the linker in building our documentation site with the locally
built version of angular.
@josephperrott josephperrott added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Feb 14, 2024
@pullapprove pullapprove bot requested a review from JoostK February 14, 2024 18:03
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

Devtools has the same use-case, where it's being made to work by ignoring the warning:

// DevTools relies on angular framework packages that are consumed,
// locally via bazel. These packages have a version of 0.0.0-PLACEHOLDER.
// DevTools also relies on Angular CDK and Material packages that are consumed via npm.
// Because of this, we set unknownDeclarationVersionHandling to ignore so that we bypass
// selecting a linker for our CDK and Material dependencies based on our local framework
// version (0.0.0-PLACEHOLDER).
// Instead this option defaults to the latest linker version, which should
// be correct, except for the small time interval where we rollout a new
// declaration version and target a material release that hasn't been compiled
// with that version yet.
unknownDeclarationVersionHandling: 'ignore',

There's also currently this logic, which I think wouldn't be needed with this change:

if (version === PLACEHOLDER_VERSION) {
// Special case if the `version` is the same as the current compiler version.
// This helps with compliance tests where the version placeholders have not been replaced.
return linkerRanges[linkerRanges.length - 1].linker;
}

EDIT: the above concerns the PLACEHOLDER version, not the substituted 0.0.0 version, so is irrelevant w.r.t. this change.

@@ -185,6 +185,11 @@ export class PartialLinkerSelector<TExpression> {
* @returns A semver range for the provided `version` and comparator.
*/
function getRange(comparator: '<='|'>=', versionStr: string): semver.Range {
// If the provided version is exactly `0.0.0` then we are known to be running with an unpublished
// version of angular and assume that all ranges are compatible.
if (versionStr === '0.0.0' && (PLACEHOLDER_VERSION as string) === '0.0.0') {
Copy link
Member

Choose a reason for hiding this comment

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

Where does the 0.0.0 come from exactly, as the version for HEAD is suffixed with -PLACEHOLDER

Copy link
Member Author

Choose a reason for hiding this comment

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

0.0.0 is stamped from during the packaging, with 0.0.0 being the replacement when we are not "stamping" and the actual version stamping when we --stamp:

angular/tools/defaults.bzl

Lines 238 to 243 in ed0d5ec

substitutions = dict(common_substitutions, **{
"0.0.0-PLACEHOLDER": "0.0.0",
})
stamped_substitutions = dict(common_substitutions, **{
"0.0.0-PLACEHOLDER": "{STABLE_PROJECT_VERSION}",
})

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks for the reference! I wasn't aware of this behavior.

@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit da7fbb4.

@josephperrott josephperrott deleted the local-linking-fix branch February 15, 2024 19:17
atscott pushed a commit to atscott/angular that referenced this pull request Feb 16, 2024
…ngular and widen supported versions (angular#54439)

When the linker is running using an unpublished version of angular, locally built, the version will be `0.0.0`.
When encountering this situation, the range that for the linker map support is considered to be `*.*.*` allowing
for the linker to work at build time with packages built with versioned angular.

Most notably this allows for us to properly use the linker in building our documentation site with the locally
built version of angular.

PR Close angular#54439
vladboisa pushed a commit to vladboisa/angular that referenced this pull request Feb 20, 2024
…ngular and widen supported versions (angular#54439)

When the linker is running using an unpublished version of angular, locally built, the version will be `0.0.0`.
When encountering this situation, the range that for the linker map support is considered to be `*.*.*` allowing
for the linker to work at build time with packages built with versioned angular.

Most notably this allows for us to properly use the linker in building our documentation site with the locally
built version of angular.

PR Close angular#54439
@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 Mar 17, 2024
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 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

3 participants