-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Webapp Angular upgrade 15 to 16 #7056
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
Changes from all commits
33f6068
3f28b0b
263e502
07fe3ff
91c5a92
2c06191
fdbafa2
892a847
a8911c2
546e9df
3d18ad4
4e3b0b7
60e6d6f
6f2123c
b984292
29f809f
a00b763
b02fda5
91c98f6
6ba9975
0d3b6ea
70f2ad4
51a7169
c2cbf17
7f530da
06c7996
5264495
1d9e029
9cfb9ec
5363ed4
ad12bd4
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 |
|---|---|---|
| @@ -1,3 +1,33 @@ | ||
| diff --git a/node_modules/@bazel/concatjs/internal/common/compilation.bzl b/node_modules/@bazel/concatjs/internal/common/compilation.bzl | ||
| index fed787a..377915a 100755 | ||
| --- a/node_modules/@bazel/concatjs/internal/common/compilation.bzl | ||
| +++ b/node_modules/@bazel/concatjs/internal/common/compilation.bzl | ||
| @@ -160,25 +160,11 @@ def _outputs(ctx, label, srcs_files = []): | ||
| closure_js_file = ctx.actions.declare_file(basename + ".mjs") | ||
| closure_js_files.append(closure_js_file) | ||
|
|
||
| - # Temporary until all imports of ngfactory/ngsummary files are removed | ||
| - # TODO(alexeagle): clean up after Ivy launch | ||
| - if getattr(ctx.attr, "use_angular_plugin", False): | ||
| - closure_js_files.append(ctx.actions.declare_file(basename + ".ngfactory.mjs")) | ||
| - closure_js_files.append(ctx.actions.declare_file(basename + ".ngsummary.mjs")) | ||
| - | ||
|
Contributor
Author
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. In this Angular version Ivy is already available so these lines can be removed. @todos resolved. 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. Do you think we should try to upgrade @bazel/concatjs to avoid having to maintain this patch file? Low priority -- im fine with this as it is now but it might be sort of kicking a can down the road that we can avoid if we just upgrade.
Contributor
Author
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. Yes, thank you @nattySP , the plan is to upgrade to the latest version of @bazel/concatjs once we migrate to Angular 21. Since we are currently on a transitional version and there is no official compatibility table for this library, patching it was the most efficient temporary solution
Member
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. Am I understanding correctly that these lines are present in the standard concatjs 5.7.0 package, and this patch is removing them? (So this TODO is likely removed in a later version of that library?) Do you know what happens if we don't remove these lines?
Contributor
Author
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. Yes, most probably this will be fixed in newer version of this patch. |
||
| if not is_dts: | ||
| devmode_js_file = ctx.actions.declare_file(basename + ".js") | ||
| devmode_js_files.append(devmode_js_file) | ||
| transpilation_infos.append(struct(closure = closure_js_file, devmode = devmode_js_file)) | ||
| declaration_files.append(ctx.actions.declare_file(basename + ".d.ts")) | ||
| - | ||
| - # Temporary until all imports of ngfactory/ngsummary files are removed | ||
| - # TODO(alexeagle): clean up after Ivy launch | ||
| - if getattr(ctx.attr, "use_angular_plugin", False): | ||
| - devmode_js_files.append(ctx.actions.declare_file(basename + ".ngfactory.js")) | ||
| - devmode_js_files.append(ctx.actions.declare_file(basename + ".ngsummary.js")) | ||
| - declaration_files.append(ctx.actions.declare_file(basename + ".ngfactory.d.ts")) | ||
| - declaration_files.append(ctx.actions.declare_file(basename + ".ngsummary.d.ts")) | ||
| return struct( | ||
| closure_js = closure_js_files, | ||
| devmode_js = devmode_js_files, | ||
| diff --git a/node_modules/@bazel/concatjs/web_test/karma.conf.js b/node_modules/@bazel/concatjs/web_test/karma.conf.js | ||
| index 90a03ef..28778c9 100755 | ||
| --- a/node_modules/@bazel/concatjs/web_test/karma.conf.js | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -224,8 +224,8 @@ def tf_ng_web_test_suite(name, deps = [], **kwargs): | |
| name = name, | ||
| bootstrap = | ||
| [ | ||
| "@npm//:node_modules/zone.js/dist/zone-evergreen.js", | ||
| "@npm//:node_modules/zone.js/dist/zone-testing.js", | ||
| "@npm//:node_modules/zone.js/bundles/zone.umd.js", | ||
| "@npm//:node_modules/zone.js/bundles/zone-testing.umd.js", | ||
| "@npm//:node_modules/reflect-metadata/Reflect.js", | ||
|
Contributor
Author
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. New paths for zone.js |
||
| ], | ||
| # The only dependency is the esbuild bundle from spec_bundle(). | ||
|
|
||
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.
Only change in this file. The newer version of @angular-devkit/build-angular: 15.2.11 (resolved in yarn.lock) explicitly added @babel/helper-split-export-declaration as its own dependency. The BUILD.bazel for the optimization scripts can not find it, so needs to be added.