-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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 browser bundle for AMD #8374
Conversation
For anybody following along at home, this doesn't fix SystemJS. I was definitely wrong to assume that. It gets detected as AMD in the outer wrapper but it's |
Much more scientific than the rest so it should stick.
function wrapperify(src) { | ||
src = src.replace('define([],f)', 'return f()'); | ||
src = src.replace( |
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.
Let's throw if there are no matches.
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.
👍 done.
* Fix browser bundle for AMD * Final fix for standalone browser build. Much more scientific than the rest so it should stick. * Throw when we can't find code we need to replace. (cherry picked from commit a3ba48b)
* Fix browser bundle for AMD * Final fix for standalone browser build. Much more scientific than the rest so it should stick. * Throw when we can't find code we need to replace.
Fixes #8301
I confirmed that the webpack bundling that got us into this situation is still ok so I think this should be safe. I didn't explicitly check SystemJS but I think it should be in the same boat.
cc @gaearon