-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(core): do not error when ngDevMode
is undeclared
#39415
Conversation
In production mode, the `ngDevMode` global may not have been declared. This is typically not a problem, as optimizers should have removed all usages of the `ngDevMode` variables. This does however require the bundler/optimizer to have been configured in a certain way, as to allow for `ngDevMode` guarded code to be removed. As an example, Terser can be configured to remove the `ngDevMode` guarded code using the following configuration: ```js const terserOptions = { // ... compress: { // ... global_defs: require('@angular/compiler-cli').GLOBAL_DEFS_FOR_TERSER, } } ``` (Taken from angular#31595 (comment)) If this is not done, however, the bundle should still work (albeit with larger code size due to missed tree-shaking opportunities). This commit adds a check for whether `ngDevMode` has been declared, as it is a top-level statement that executes before `ngDevMode` has been initialized. Fixes angular#31595
@@ -38,7 +38,7 @@ export {Sanitizer} from './sanitization/sanitizer'; | |||
export * from './codegen_private_exports'; | |||
|
|||
import {global} from './util/global'; | |||
if (ngDevMode) { | |||
if (typeof ngDevMode !== 'undefined' && ngDevMode) { |
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.
Drive-by observation: the typeof
check is something that can easily be missed. Maybe we can add a custom lint rule to check for it?
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.
I agree this is indeed error-prone and easy to regress in some way or another. There are however plenty of bare reads of ngDevMode
in places that can depend on initNgDevMode
to have been executed. A potential lint rule would therefore likely have to only consider top-level statements or something like that, which would also make it not entirely fool proof.
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
In production mode, the `ngDevMode` global may not have been declared. This is typically not a problem, as optimizers should have removed all usages of the `ngDevMode` variables. This does however require the bundler/optimizer to have been configured in a certain way, as to allow for `ngDevMode` guarded code to be removed. As an example, Terser can be configured to remove the `ngDevMode` guarded code using the following configuration: ```js const terserOptions = { // ... compress: { // ... global_defs: require('@angular/compiler-cli').GLOBAL_DEFS_FOR_TERSER, } } ``` (Taken from #31595 (comment)) If this is not done, however, the bundle should still work (albeit with larger code size due to missed tree-shaking opportunities). This commit adds a check for whether `ngDevMode` has been declared, as it is a top-level statement that executes before `ngDevMode` has been initialized. Fixes #31595 PR Close #39415
In production mode, the `ngDevMode` global may not have been declared. This is typically not a problem, as optimizers should have removed all usages of the `ngDevMode` variables. This does however require the bundler/optimizer to have been configured in a certain way, as to allow for `ngDevMode` guarded code to be removed. As an example, Terser can be configured to remove the `ngDevMode` guarded code using the following configuration: ```js const terserOptions = { // ... compress: { // ... global_defs: require('@angular/compiler-cli').GLOBAL_DEFS_FOR_TERSER, } } ``` (Taken from #31595 (comment)) If this is not done, however, the bundle should still work (albeit with larger code size due to missed tree-shaking opportunities). This commit adds a check for whether `ngDevMode` has been declared, as it is a top-level statement that executes before `ngDevMode` has been initialized. Fixes #31595 PR Close #39415
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. |
In production mode, the
ngDevMode
global may not have been declared.This is typically not a problem, as optimizers should have removed all
usages of the
ngDevMode
variables. This does however require thebundler/optimizer to have been configured in a certain way, as to allow
for
ngDevMode
guarded code to be removed.As an example, Terser can be configured to remove the
ngDevMode
guarded code using the following configuration:
(Taken from #31595 (comment))
If this is not done, however, the bundle should still work (albeit with
larger code size due to missed tree-shaking opportunities). This commit
adds a check for whether
ngDevMode
has been declared, as it is atop-level statement that executes before
ngDevMode
has been initialized.Fixes #31595