-
Notifications
You must be signed in to change notification settings - Fork 11.9k
fix(@angular/build): correct misleading error message for top-level await #32887
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
base: main
Are you sure you want to change the base?
Changes from all commits
8c9238e
12d555f
37db804
9583d87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import { extractLicenses } from '../../tools/esbuild/license-extractor'; | |
| import { profileAsync } from '../../tools/esbuild/profiling'; | ||
| import { | ||
| calculateEstimatedTransferSizes, | ||
| isZonelessApp, | ||
| logBuildStats, | ||
| transformSupportedBrowsersToTargets, | ||
| } from '../../tools/esbuild/utils'; | ||
|
|
@@ -37,6 +38,10 @@ import { inlineI18n, loadActiveTranslations } from './i18n'; | |
| import { NormalizedApplicationBuildOptions } from './options'; | ||
| import { createComponentStyleBundler, setupBundlerContexts } from './setup-bundling'; | ||
|
|
||
| /** The esbuild error text prefix used to detect top-level await errors. */ | ||
| const TOP_LEVEL_AWAIT_ERROR_TEXT = | ||
| 'Top-level await is not available in the configured target environment'; | ||
|
|
||
| // eslint-disable-next-line max-lines-per-function | ||
| export async function executeBuild( | ||
| options: NormalizedApplicationBuildOptions, | ||
|
|
@@ -156,6 +161,29 @@ export async function executeBuild( | |
|
|
||
| // Return if the bundling has errors | ||
| if (bundlingResult.errors) { | ||
| // If Zone.js is used, augment top-level await errors with a more helpful message. | ||
| // esbuild's default error mentions "target environment" with browser versions, but | ||
| // the actual reason is that async/await is downleveled for Zone.js compatibility. | ||
| if (!isZonelessApp(options.polyfills)) { | ||
| for (const error of bundlingResult.errors) { | ||
| if (error.text?.startsWith(TOP_LEVEL_AWAIT_ERROR_TEXT)) { | ||
| error.notes = [ | ||
| { | ||
| text: | ||
| 'Top-level await is not supported in applications that use Zone.js. ' + | ||
| 'Consider removing Zone.js or moving this code into an async function.', | ||
| location: null, | ||
| }, | ||
| { | ||
| text: 'For more information about zoneless Angular applications, visit: https://angular.dev/guide/zoneless', | ||
| location: null, | ||
| }, | ||
| ...(error.notes ?? []), | ||
| ]; | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+167
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To improve maintainability and readability, it's a good practice to extract magic strings into constants. The error message from esbuild could change in the future, and having it as a constant makes it easier to update. if (!isZonelessApp(options.polyfills)) {
const TLA_ERROR_MESSAGE =
'Top-level await is not available in the configured target environment';
for (const error of bundlingResult.errors) {
if (error.text?.startsWith(TLA_ERROR_MESSAGE)) {
error.notes = [
{
text:
'Top-level await is not supported in applications that use Zone.js. ' +
'Consider removing Zone.js or moving this code into an async function.',
location: null,
},
{
text: 'For more information about zoneless Angular applications, visit: https://angular.dev/guide/zoneless',
location: null,
},
...(error.notes ?? []),
];
}
}
} |
||
|
|
||
| executionResult.addErrors(bundlingResult.errors); | ||
|
|
||
| return executionResult; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * @license | ||||||||||||||||||||||||||||||||
| * Copyright Google LLC All Rights Reserved. | ||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||
| * Use of this source code is governed by an MIT-style license that can be | ||||||||||||||||||||||||||||||||
| * found in the LICENSE file at https://angular.dev/license | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import { buildApplication } from '../../index'; | ||||||||||||||||||||||||||||||||
| import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { | ||||||||||||||||||||||||||||||||
| describe('Behavior: "Top-level await error message"', () => { | ||||||||||||||||||||||||||||||||
| it('should show a Zone.js-specific error when top-level await is used with Zone.js', async () => { | ||||||||||||||||||||||||||||||||
| await harness.writeFile( | ||||||||||||||||||||||||||||||||
| 'src/main.ts', | ||||||||||||||||||||||||||||||||
| ` | ||||||||||||||||||||||||||||||||
| const value = await Promise.resolve('test'); | ||||||||||||||||||||||||||||||||
| console.log(value); | ||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| harness.useTarget('build', { | ||||||||||||||||||||||||||||||||
| ...BASE_OPTIONS, | ||||||||||||||||||||||||||||||||
| polyfills: ['zone.js'], | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false }); | ||||||||||||||||||||||||||||||||
| expect(result?.success).toBeFalse(); | ||||||||||||||||||||||||||||||||
| expect(logs).toContain( | ||||||||||||||||||||||||||||||||
| jasmine.objectContaining({ | ||||||||||||||||||||||||||||||||
| message: jasmine.stringMatching( | ||||||||||||||||||||||||||||||||
| 'Top-level await is not supported in applications that use Zone.js', | ||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| it('should not show a Zone.js-specific error when top-level await is used without Zone.js', async () => { | ||||||||||||||||||||||||||||||||
| await harness.writeFile( | ||||||||||||||||||||||||||||||||
| 'src/main.ts', | ||||||||||||||||||||||||||||||||
| ` | ||||||||||||||||||||||||||||||||
| const value = await Promise.resolve('test'); | ||||||||||||||||||||||||||||||||
| console.log(value); | ||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| harness.useTarget('build', { | ||||||||||||||||||||||||||||||||
| ...BASE_OPTIONS, | ||||||||||||||||||||||||||||||||
| polyfills: [], | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false }); | ||||||||||||||||||||||||||||||||
| // Without Zone.js, the build may still fail due to target environment constraints, | ||||||||||||||||||||||||||||||||
| // but the error should NOT contain the Zone.js-specific message | ||||||||||||||||||||||||||||||||
| const zoneJsErrorPresent = logs.some( | ||||||||||||||||||||||||||||||||
| (log) => | ||||||||||||||||||||||||||||||||
| typeof log.message === 'string' && | ||||||||||||||||||||||||||||||||
| log.message.includes('Top-level await is not supported in applications that use Zone.js'), | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| expect(zoneJsErrorPresent).toBeFalse(); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+56
to
+61
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.