Skip to content

build: fix release output containing invalid imports sometimes #20214

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

Merged

Conversation

devversion
Copy link
Member

Resubmit of #20201 with an updated patch. Still not 100% confident why the TS compilation failed, but investigation
showed that the previous patch using a Proxy meant that TS sometimes is unable to read the moduleName
compared to it being an actual property on the source file object. Most likely TS internally tries to read
the module name, but it's not defined on the source file, but in the actual Proxy object.

A very subtle issue has surfaced in our release output. Based on
previous builds that have been performed locally by the caretaker
performing the release, the release output might differ and contain
invalid module imports that break ES2015 output.

Notably the es2015 output is not primarily used by the CLI. It uses
the flat ES2015 output instead.

The issue surfaces because of a bug in `@bazel/typescript` where the
`moduleName` for source files is leaking between TSC worker runs. This
means that source files are incorrectly referenced by module name,
instead of relative path when the prodmode ES2015 output is built.
Deep paths to source files are not supported as per Angular Package format.

We patch the fix that is pending upstream to ensure our release output
works: bazelbuild/rules_typescript#504. Once the
fix upstream lands (which might take quite some time; given g3 sync and
sync with the rules_nodejs repository), we can remove the patch and
update to the latest version of `@bazel/typescript`.

Fixes angular#20179.
@devversion devversion added P2 The issue is important to a large percentage of users, with a workaround merge safe target: patch This PR is targeted for the next patch release labels Aug 6, 2020
@devversion devversion requested a review from a team as a code owner August 6, 2020 09:41
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 6, 2020
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

@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Aug 6, 2020
@andrewseguin andrewseguin merged commit f6e82cc into angular:master Aug 12, 2020
andrewseguin pushed a commit that referenced this pull request Aug 12, 2020
A very subtle issue has surfaced in our release output. Based on
previous builds that have been performed locally by the caretaker
performing the release, the release output might differ and contain
invalid module imports that break ES2015 output.

Notably the es2015 output is not primarily used by the CLI. It uses
the flat ES2015 output instead.

The issue surfaces because of a bug in `@bazel/typescript` where the
`moduleName` for source files is leaking between TSC worker runs. This
means that source files are incorrectly referenced by module name,
instead of relative path when the prodmode ES2015 output is built.
Deep paths to source files are not supported as per Angular Package format.

We patch the fix that is pending upstream to ensure our release output
works: bazelbuild/rules_typescript#504. Once the
fix upstream lands (which might take quite some time; given g3 sync and
sync with the rules_nodejs repository), we can remove the patch and
update to the latest version of `@bazel/typescript`.

Fixes #20179.

(cherry picked from commit f6e82cc)
@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 Sep 12, 2020
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 P2 The issue is important to a large percentage of users, with a workaround 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