Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

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

Merged
merged 4 commits into from Sep 19, 2022

Conversation

danielroe
Copy link
Member

πŸ”— Linked issue

resolves nuxt/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.

@danielroe danielroe 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 pi0 September 17, 2022 10:52
@danielroe danielroe self-assigned this Sep 17, 2022
@netlify
Copy link

netlify bot commented Sep 17, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit b304b67
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6325abddff8ba60008fcd1be

@danielroe danielroe added the vite label Sep 17, 2022
@pi0
Copy link
Member

pi0 commented Sep 19, 2022

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
Member Author

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 merged commit f536bf5 into main Sep 19, 2022
@pi0 pi0 deleted the fix/webpack-keys branch September 19, 2022 09:34
@pi0 pi0 mentioned this pull request Sep 20, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x bug Something isn't working ❗ p4-important Priority 4: bugs that violate documented behavior, or significantly impact perf performance vite webpack
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