Skip to content

Conversation

@benmccann
Copy link
Collaborator

Description

Credit to @Rich-Harris for this. He noticed that the casing was wrong here:

'crossorigin' in document.createElement('link'); // false
'crossOrigin' in document.createElement('link'); // true

Also renaming the variable here since the function is only called with a link and not a script

Additional context

Even with the fix, the feature still seems pretty broken: #5532


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@benmccann benmccann added bug p3-minor-bug An edge case that only affects very specific usage (priority) labels Feb 11, 2023
bluwy
bluwy previously approved these changes Feb 13, 2023
@bluwy
Copy link
Member

bluwy commented Feb 13, 2023

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Feb 13, 2023

📝 Ran ecosystem CI: Open

suite result
astro ✅ success
histoire ✅ success
iles ✅ success
ladle ✅ success
laravel ✅ success
marko ✅ success
nuxt-framework ✅ success
previewjs ✅ success
qwik ✅ success
rakkas ✅ success
sveltekit ✅ success
vite-plugin-ssr ✅ success
vite-plugin-react ✅ success
vite-plugin-react-pages ✅ success
vite-plugin-react-swc ✅ success
vite-plugin-svelte ✅ success
vite-plugin-vue ✅ success
vite-setup-catalogue ❌ failure
vitepress ✅ success
vitest ✅ success
windicss ✅ success

if (script.referrerpolicy) fetchOpts.referrerPolicy = script.referrerpolicy
if (script.crossorigin === 'use-credentials')
if (link.integrity) fetchOpts.integrity = link.integrity
if (link.referrerpolicy) fetchOpts.referrerPolicy = link.referrerpolicy
Copy link
Member

@sapphi-red sapphi-red Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (link.referrerpolicy) fetchOpts.referrerPolicy = link.referrerpolicy
if (link.referrerPolicy) fetchOpts.referrerPolicy = link.referrerPolicy
'referrerPolicy' in document.createElement('link') // true
'referrerpolicy' in document.createElement('link') // false

It seems this one is also wrong. (applied this change)

@sapphi-red
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Feb 14, 2023

📝 Ran ecosystem CI: Open

suite result
astro ✅ success
histoire ✅ success
iles ✅ success
ladle ✅ success
laravel ✅ success
marko ✅ success
nuxt-framework ✅ success
previewjs ✅ success
qwik ✅ success
rakkas ✅ success
sveltekit ✅ success
vite-plugin-ssr ✅ success
vite-plugin-react ✅ success
vite-plugin-react-pages ✅ success
vite-plugin-react-swc ✅ success
vite-plugin-svelte ✅ success
vite-plugin-vue ✅ success
vite-setup-catalogue ❌ failure
vitepress ✅ success
vitest ✅ success
windicss ✅ success

@bluwy bluwy merged commit 6a0d356 into main Feb 14, 2023
@bluwy bluwy deleted the benmccann-patch-2 branch February 14, 2023 09:17
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants