X Tutup
Skip to content

build(devtools): APF fixes #50086

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

Closed
AleksanderBodurri wants to merge 2 commits into from
Closed

build(devtools): APF fixes #50086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df30710
build(devtools): make sure linker runs on fesm2022 bundles
AleksanderBodurri May 1, 2023
3d3faec
build(devtools): target es2020 explicitly
AleksanderBodurri May 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions devtools/projects/shell-browser/src/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ esbuild(
minify = True,
platform = "browser",
splitting = False,
target = "esnext",
# todo(aleksanderbodurri): here we target es2020 explicitly.
# We do this because of a bug caused by https://github.com/evanw/esbuild/issues/2950 and an Angular v16 change
# to how angular static properties are attached to class constructors.
# Targeting esnext or es2022 will cause the static initializer blocks that attach these static properties on class
# constructors to reference a class constructor variable that they do not have access to.
target = "es2020",
deps = LINKER_PROCESSED_FW_PACKAGES + [":src"],
)

Expand All @@ -76,7 +81,12 @@ esbuild(
minify = True,
platform = "browser",
splitting = False,
target = "esnext",
# todo(aleksanderbodurri): here we target es2020 explicitly.
# We do this because of a bug caused by https://github.com/evanw/esbuild/issues/2950 and an Angular v16 change
# to how angular static properties are attached to class constructors.
# Targeting esnext or es2022 will cause the static initializer blocks that attach these static properties on class
# constructors to reference a class constructor variable that they do not have access to.
target = "es2020",
Copy link
Copy Markdown
Member

@devversion devversion May 2, 2023

Choose a reason for hiding this comment

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

Have we considered shipping ES2022 here?

Copy link
Copy Markdown
Member Author

@AleksanderBodurri AleksanderBodurri May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the second commit description explains why this is shipping es2020 instead of es2022.

Starting with v16, static properties on component constructors are initialized from static initializer blocks,

class MyCmp {
    static { this.ɵcmp = i0.ɵɵngDeclareComponent({ type: MyCmp, ... }); }
}

esbuild has a known bug evanw/esbuild#2950 that causes builds with the es2022 target to transform the above into

var MyCmp = class {
    static { this.ɵcmp = i0.ɵɵngDeclareComponent({ type: MyCmp, ... }); }
}

In the static initialization block above, we don't actually have access toMyCmp like we do in the pre-esbuild transformation step above that one.

Here's a useful snippet that you can paste directly in a browser console to see an example of this,

var MyCmp = class { static { console.log(MyCmp) } }

Copy link
Copy Markdown
Member

@devversion devversion May 2, 2023

Choose a reason for hiding this comment

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

Ops. Thanks. appreciate you putting that into the commit message. I'd prefer linking a TODO in the code, but optional nit.

Copy link
Copy Markdown
Member Author

@AleksanderBodurri AleksanderBodurri May 2, 2023

Choose a reason for hiding this comment

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

Good point, I'll leave a todo. Thanks Paul 🙏

Copy link
Copy Markdown
Member

@clydin clydin May 3, 2023

Choose a reason for hiding this comment

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

You potentially could still target es2022, if preferred, by individually turning off support for class static blocks.
esbuild has a configuration option for this:

  supported: {
    'class-static-blocks': false,
  },

deps = [":devtools"],
)

Expand Down
7 changes: 6 additions & 1 deletion devtools/src/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ esbuild(
entry_points = [":main.ts"],
platform = "browser",
splitting = True,
target = "es2016",
# todo(aleksanderbodurri): here we target es2020 explicitly.
# We do this because of a bug caused by https://github.com/evanw/esbuild/issues/2950 and an Angular v16 change
# to how angular static properties are attached to class constructors.
# Targeting esnext or es2022 will cause the static initializer blocks that attach these static properties on class
# constructors to reference a class constructor variable that they do not have access to.
target = "es2020",
deps = LINKER_PROCESSED_FW_PACKAGES + [":demo"],
)

Expand Down
4 changes: 2 additions & 2 deletions devtools/tools/esbuild/esbuild-base.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ export default async function createConfig({enableLinker, optimize}) {
enableLinker: enableLinker
? {
ensureNoPartialDeclaration: false,
// Only run the linker on `fesm2020/` bundles. This should not have an effect on
// Only run the linker on fesm2020 and fesm2022 bundles. This should not have an effect on
// the bundle output, but helps speeding up ESBuild when it visits other modules.
filterPaths: /fesm2020/,
filterPaths: /fesm2020|fesm2022/,
linkerOptions: {
// DevTools relies on angular framework packages that are consumed,
// locally via bazel. These packages have a version of 0.0.0-PLACEHOLDER.
Expand Down
X Tutup