-
Notifications
You must be signed in to change notification settings - Fork 26.2k
fix(ivy): ensure DebugNode
/DebugElement
are tree-shakeable in Ivy
#35003
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
Conversation
You can preview a50c40e at https://pr35003-a50c40e.ngbuilds.io/. |
a50c40e
to
02e8ef6
Compare
DebugNode
/DebugElement
are tree-shakeable in Ivy
You can preview 02e8ef6 at https://pr35003-02e8ef6.ngbuilds.io/. |
02e8ef6
to
47710b0
Compare
return null; | ||
} | ||
|
||
export const getDebugNodeR2: (nativeNode: any) => DebugNode | null = getDebugNodeR2__PRE_R3__; |
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.
Not sure what would be an appropriate name 😇
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
47710b0
to
3fccd85
Compare
You can preview 3fccd85 at https://pr35003-3fccd85.ngbuilds.io/. |
There are different `DebugNode`/`DebugElement` implementations (and associated helper functions) for ViewEngine and Ivy. Additionally, these classes/functions, which are defined inside the `core` package, are imported by the `platform-browser` package. Previously, this code was not tree-shaken as expected in Ivy. angular#30130 partially addressed the issue, but only for the case where `core` and `platform-browser` end up in the same closure after webpack's scope hoisting. In cases where this is not the case, our webpack/terser based tooling is not capable of tree-shaking it. This commit fixes the problem, by ensuring that the code retained in Ivy mode (due to the cross-package import) does not unnecessarily reference `DebugNode`/`DebugElement`, allowing the code to be tree-shaken away. This results in a 7.6KB reduction in the size of the main angular.io bundle. Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802)
0bd5ed3
to
9c7d3dd
Compare
You can preview 9c7d3dd at https://pr35003-9c7d3dd.ngbuilds.io/. |
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.
George, you rock!
thanks for figuring this out.
❤️
@@ -12,7 +12,7 @@ | |||
"master": { | |||
"uncompressed": { | |||
"runtime-es2015": 2987, | |||
"main-es2015": 456604, | |||
"main-es2015": 448956, |
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.
woot!!!
@@ -39,7 +39,7 @@ | |||
"master": { | |||
"uncompressed": { | |||
"runtime-es2015": 2289, | |||
"main-es2015": 254657, | |||
"main-es2015": 247225, |
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.
sweet!
return null; | ||
} | ||
|
||
export const getDebugNodeR2: (nativeNode: any) => DebugNode | null = getDebugNodeR2__PRE_R3__; |
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
…#35003) There are different `DebugNode`/`DebugElement` implementations (and associated helper functions) for ViewEngine and Ivy. Additionally, these classes/functions, which are defined inside the `core` package, are imported by the `platform-browser` package. Previously, this code was not tree-shaken as expected in Ivy. #30130 partially addressed the issue, but only for the case where `core` and `platform-browser` end up in the same closure after webpack's scope hoisting. In cases where this is not the case, our webpack/terser based tooling is not capable of tree-shaking it. This commit fixes the problem, by ensuring that the code retained in Ivy mode (due to the cross-package import) does not unnecessarily reference `DebugNode`/`DebugElement`, allowing the code to be tree-shaken away. This results in a 7.6KB reduction in the size of the main angular.io bundle. Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802) PR Close #35003
…#35003) There are different `DebugNode`/`DebugElement` implementations (and associated helper functions) for ViewEngine and Ivy. Additionally, these classes/functions, which are defined inside the `core` package, are imported by the `platform-browser` package. Previously, this code was not tree-shaken as expected in Ivy. #30130 partially addressed the issue, but only for the case where `core` and `platform-browser` end up in the same closure after webpack's scope hoisting. In cases where this is not the case, our webpack/terser based tooling is not capable of tree-shaking it. This commit fixes the problem, by ensuring that the code retained in Ivy mode (due to the cross-package import) does not unnecessarily reference `DebugNode`/`DebugElement`, allowing the code to be tree-shaken away. This results in a 7.6KB reduction in the size of the main angular.io bundle. Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802) PR Close #35003
…angular#35003) There are different `DebugNode`/`DebugElement` implementations (and associated helper functions) for ViewEngine and Ivy. Additionally, these classes/functions, which are defined inside the `core` package, are imported by the `platform-browser` package. Previously, this code was not tree-shaken as expected in Ivy. angular#30130 partially addressed the issue, but only for the case where `core` and `platform-browser` end up in the same closure after webpack's scope hoisting. In cases where this is not the case, our webpack/terser based tooling is not capable of tree-shaking it. This commit fixes the problem, by ensuring that the code retained in Ivy mode (due to the cross-package import) does not unnecessarily reference `DebugNode`/`DebugElement`, allowing the code to be tree-shaken away. This results in a 7.6KB reduction in the size of the main angular.io bundle. Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802) PR Close angular#35003
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. |
There are different
DebugNode
/DebugElement
implementations (and associated helper functions) for ViewEngine and Ivy. Additionally, these classes/functions, which are defined inside thecore
package, are imported by theplatform-browser
package.Previously, this code was not tree-shaken as expected in Ivy. #30130 partially addressed the issue, but only for the case where
core
andplatform-browser
end up in the same closure after webpack's scope hoisting. In cases where this is not the case, our webpack/terser based tooling is not capable of tree-shaking it.This commit fixes the problem, by ensuring that the code retained in Ivy mode (due to the cross-package import) does not unnecessarily reference
DebugNode
/DebugElement
, allowing the code to be tree-shaken away. This results in a 7.6KB reduction in the size of the main angular.iobundle.
Jira issue: FW-1802