Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.
/frameworkPublic archive

fix(vite, webpack): avoid generating keys where a key is already provided - #7622

Merged
pi0 merged 4 commits into
mainfrom
fix/webpack-keys
Sep 19, 2022
Merged

fix(vite, webpack): avoid generating keys where a key is already provided#7622
pi0 merged 4 commits into
mainfrom
fix/webpack-keys

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

resolvesnuxt/nuxt#14932

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

With webpack, we were processing code twice and therefore injecting several auto-generated keys, one after the other. This PR prevents double-insertion of keys, and also prevents insertion when there is an obvious string or template literal key provided already.

It also excludes nuxt sourcefiles (e.g. #app) from having keys inserted into them.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroedanielroe added bug Something isn't working webpack ❗ p4-important Priority 4: bugs that violate documented behavior, or significantly impact perf performance labels Sep 17, 2022
@danielroe
danielroe requested a review from pi0September 17, 2022 10:52
@danielroedanielroe self-assigned this Sep 17, 2022
@netlify

netlifyBot commented Sep 17, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs canceled.

NameLink
🔨 Latest commitb304b67
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/6325abddff8ba60008fcd1be

@pi0

pi0 commented Sep 19, 2022

Copy link
Copy Markdown
Member

I love this as workaround but also it can be even trickier in the future when we support more arguments (like 3rd options to useState) and if first arg is not a string but variable/reference. Thinking if we could change auto key injection method to be safe out of the box.

@danielroe

Copy link
Copy Markdown
MemberAuthor

You're right that we would need to update if we add more args, but the main thing it is protecting against is duplicate string keys at the end of the argument chain, which is what causes the issue.

Detecting explicit keys at the beginning is only an added benefit - not a bug fix.

pi0
pi0 approved these changes Sep 19, 2022
@pi0
pi0 merged commit f536bf5 into mainSep 19, 2022
@pi0
pi0 deleted the fix/webpack-keys branch September 19, 2022 09:34
@pi0pi0 mentioned this pull request Sep 20, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.xbugSomething isn't working❗ p4-importantPriority 4: bugs that violate documented behavior, or significantly impact perfperformancevitewebpack

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[nitro] [dev] [unhandledRejection] TypeError: Cannot create property 'server' on string '$KxdiLPLTek'

2 participants

@danielroe@pi0