-
Notifications
You must be signed in to change notification settings - Fork 6.8k
build: validate bazel rollup globals #17173
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
build: validate bazel rollup globals #17173
Conversation
c4ed87d
to
1cd823b
Compare
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.
LGTM
15a8f12
to
0ffe8f0
Compare
Rebased since the |
0ffe8f0
to
461ceb9
Compare
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.
LGTM, just one nit
We can't import individual files through the module name because: * We don't have these listed in the rollup globals * These imports will break in the Angular package (due to limitation in APF).
Apparently the rollup globals tslint rule did not check all files. Resuling in a few cases where the rollup globals were missing.
We currently need to also maintain a list of rollup globals for bazel. Since these globals are impacting the release output we need to check these somehow in order to be sure that we do not accidentally bundle other entry-points into an entry-point.
461ceb9
to
e19092e
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Note: to reduce too much changes to the gulp build system yet, the rollup globals tslint rule will remain for now.