-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
feat: add crossOriginLoading option support #313
Conversation
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
crossOriginLoading
is truthy, adds thecrossOrigin
attribute - remove the
if
guard
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 |
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