-
-
Notifications
You must be signed in to change notification settings - Fork 388
feat: add crossOriginLoading option support #313
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
ooflorent
left a comment
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 like the idea on reusing crossOriginLoading option but I don't like the way it is integrated in the runtime code.
In my opinion it would be better to do it like webpack.
- if
crossOriginLoadingis truthy, adds thecrossOriginattribute - remove the
ifguard
In webpack, the runtime generation is different but you can look at the implementation here: https://github.com/webpack/webpack/blob/614a6fc0109a702ee64ca0b0982a0d638b1b6ca1/lib/web/JsonpMainTemplate.runtime.js#L19
|
@ooflorent In webpack it's similar. Also guarded by an if. It makes sure that cookies are send when requesting same-origin. see also webpack/webpack#7017 |
alexander-akait
left a comment
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.
Good work, can we add test? Maybe manual
42fd988 to
273e2a8
Compare
|
Please accept CLA |
|
@evilebottnawi done |
|
@chikara-chan something wrong, looks you have difference emails in commits and in CLA |
|
@evilebottnawi Sign the CLA with the same email in success! |
This PR contains a:
Motivation / Use-Case
#312
Reference code. We could reuse Webpack output options.
Breaking Changes
Additional Info