fix: allow cross origin loading for graphiql #775
Conversation
| @@ -6,6 +6,7 @@ const importer = { | |||
| const script = document.createElement('script') | |||
| script.type = 'text/javascript' | |||
| script.src = url | |||
| script.crossOrigin = 'anonymous' | |||
There was a problem hiding this comment.
Minor point, but the actual name of the property is 'crossorigin' (all lower case).
I know the camel case version works, and I know that html attribute names are case-insensitive, but I think I'd prefer to use the casing indicated by the spec.
There was a problem hiding this comment.
According to MDN, the proper name is crossOrigin when we refer to it from JS https://developer.mozilla.org/en-US/docs/Web/API/HTMLScriptElement
The fix won't work if the casing is changed to all lower case.
|
@conradthegray can you please link the issue properly in the PR description? |
|
Build is failing but I don't see anything in this PR which may cause that. On the other hand the build is green on master and it doesn't have a history of failing, so I'm confused what's going on |
|
I'll go ahead and merge this. If the failure occurs on master, we'll look into it straight away |
|
yeah that's very strange, I also can't see how this change would affect the tests. Looking at the file in question, it seems like this was initially observed when creating the relevant PR but the failures seemed to be fixed in later commits: #706 (comment) . To me, this suggests that it is intermittent so maybe a rerun of the jobs will be okay - like you mentioned, if it happens on master, we can take a closer look! |
|
master build is green 🤷 |
Fixes #771
unpkg is not setting up correct cross origin headers causing browsers to block scripts and styles loaded from them. The proposed change tells the browser to load the content from unpkg.
How to test the fix
http://localhost:3000/graphiqlmercurius-771/node_modules/mercurius/static/main.jsfilehttp://localhost:3000/graphiql