Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
implement
amd.autoId
/amd.basePath
options #3867implement
amd.autoId
/amd.basePath
options #3867Changes from 14 commits
cef70d8
a44bef3
eadea09
462c69c
287769a
65f0921
3a42519
04ec70f
3bcd1cd
02b8f21
2e9d487
cb76695
881b932
e3cf09c
95d605f
96088e2
bde40bc
ad09886
114364f
9434622
8eeece1
1654ded
1d4efc9
84e8099
96ea9c1
c9ec971
48d01b1
e1d9173
e6944ae
9cecce3
6ef8520
bba3d17
cbae7b8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 a big fan of internal errors that are impossible to test, but there are other opinions out there as well. The price of course are non-null assertions. In a future version of Rollup, maybe we can move away from classes towards data types that evolve as the bundle evolves.
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.
Yeh I had that initially but noticed a few more internal errors. Happy to remove if you want
sounds good
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 know you use this to allow
UMD
here but really I do not think it makes a lot of sense considering UMD just does not support code-splitting. Instead, use a regular form test for the non-code-splitting case.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.
Your test names are switched: The code-split test is not doing the code splitting while the other one does
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 does not call the function. The correct syntax is
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.
You will find that
exports
is undefined for you. The reason is that indeed even though it loads themain
chunk, it does not load the main module because the id does not match. If you add logs to your code, you will see that no code is executed.To fix this, we should probably extend the runAmd object so that you can specify also the id of the main entry if it is not
main
, but it will require some more digging how to handle this in requirejs.