-
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(zone.js): add conditional exports to zone.js package #51652
fix(zone.js): add conditional exports to zone.js package #51652
Conversation
fda742a
to
f5d1915
Compare
b25b6ca
to
1ed0b00
Compare
1097ede
to
ea5b7b7
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!
This is needed to better support native ESM modules and avoid the otherwise necessary deep imports like `zone.js/fesm2015/zone-node.js` due to disallowed directory imports.
ea5b7b7
to
2e49034
Compare
This commit update the lock file to force a new `node_modules` cache as this causes the patch to fail due to GitHub caching and at present, there is also no way to provide a key to invalidate the cache.
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
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.
Nice, only thing that will be a little slower is having to build the full zone npm package for benchmarks- but I think that is reasonable
This PR was merged into the repository by commit cfe2f1d. |
This commit update the lock file to force a new `node_modules` cache as this causes the patch to fail due to GitHub caching and at present, there is also no way to provide a key to invalidate the cache. PR Close #51652
"default": "./package.json" | ||
}, | ||
".": { | ||
"typings": "./zone.d.ts", |
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.
this must say "types"
for typescript to pick it up when project uses "moduleResolution"
that respects "exports"
(e.g. moduleResolution": "bundler"
).
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.
Thanks for bringing this up. Here's a fix #51737
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. |
This is needed to better support native ESM modules and avoid the otherwise necessary deep imports like `zone.js/fesm2015/zone-node.js` due to disallowed directory imports. PR Close angular#51652
This commit update the lock file to force a new `node_modules` cache as this causes the patch to fail due to GitHub caching and at present, there is also no way to provide a key to invalidate the cache. PR Close angular#51652
This is needed to better support native ESM modules and avoid the otherwise necessary deep imports like
zone.js/fesm2015/zone-node.js
due to disallowed directory imports.