Skip to content
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

fix(core): codegen should generate unique route prop filenames #10131

Merged
merged 2 commits into from
May 10, 2024

Conversation

slorber
Copy link
Collaborator

@slorber slorber commented May 10, 2024

Motivation

Fix #10125

When using docs routeBasePath: '/' and a category with slug: '/', we end up with 2 prop module files named index.json in the plugin props dir, and one overrides the other leading to issues.

To solve this, we now always append a hash (previously there was an exception for path / returning index) that depends on both the route path but also the component name. It makes it much less likely to have a filename conflict.

Note: it's still technically possible to have a conflict. A more robust solution could be to hash the props object content, but it's slower. The current solution is good enough and fast.

Test Plan

Unit test

Plus local test on the bug repro works:

CleanShot 2024-05-10 at 18 03 32@2x

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label May 10, 2024
@slorber slorber requested a review from Josh-Cena as a code owner May 10, 2024 16:02
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 10, 2024
Copy link

netlify bot commented May 10, 2024

[V2]

Name Link
🔨 Latest commit 9b9ece5
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/663e451fbd93a600096a14ae
😎 Deploy Preview https://deploy-preview-10131--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 70 🟢 98 🟢 96 🟢 100 🟠 88 Report
/docs/installation 🟠 63 🟢 96 🟢 100 🟢 100 🟠 88 Report
/docs/category/getting-started 🟠 77 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog 🟠 67 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 59 🟢 96 🟢 100 🟢 100 🟠 88 Report
/blog/tags/release 🟠 70 🟢 100 🟢 100 🟠 80 🟠 88 Report
/blog/tags 🟠 76 🟢 100 🟢 100 🟢 90 🟠 88 Report

Copy link

Size Change: -33 B (0%)

Total Size: 1.71 MB

Filename Size Change
website/build/assets/js/main.********.js 853 kB -33 B (0%)
ℹ️ View Unchanged
Filename Size
website/.docusaurus/codeTranslations.json 2 B
website/.docusaurus/docusaurus.config.mjs 26.9 kB
website/.docusaurus/globalData.json 107 kB
website/.docusaurus/i18n.json 930 B
website/.docusaurus/registry.js 275 kB
website/.docusaurus/routes.js 179 kB
website/.docusaurus/routesChunkNames.json 119 kB
website/.docusaurus/site-metadata.json 2.17 kB
website/build/assets/css/styles.********.css 112 kB
website/build/index.html 38.1 kB

compressed-size-action

@slorber slorber merged commit 29b7a4d into main May 10, 2024
32 checks passed
@slorber slorber deleted the slorber/fix-route-module-props-filename-conflict branch May 10, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failed after upgrade to 3.3.2 with a _category_.json contain slug attribute
2 participants