fix(docs): resolve metadata paths with zero-width spaces#12044
fix(docs): resolve metadata paths with zero-width spaces#12044danyalahmed1995 wants to merge 1 commit into
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
3eae039 to
452c4e4
Compare
There was a problem hiding this comment.
Honestly, this looks like AI slop
Maybe it's a legit fix, but you are not testing the problem end-to-end, and only adding tests for the code you introduced, which doesn't prove anything in practice.
It remains to be proved that the problem is actually solved, and that your PR is not introducing useless code, despite it being "tested."
Before even thinking about implementing a solution, I want to see an end-to-end test case that actually fails on our CI.
Please add a dogfooding test case to our own website, for example, as I did here: #12052
- If I remove your code, the CI must fail with the initial bug report error.
- If I add back your code, the CI must pass, proving that your implementation solves the problem.
Also, this issue is likely to also affect other plugins, so we'd rather implement a generic solution.
|
Honestly yeah i got invested in proving my approach instead of looking at the original end to end failure , I will keep that in mind apologies for that 😅. I am gonna do the testing you asked me to do and come back with a generic solution that doesn't affect other plugins. Thank you for the feedback. |
|
@slorber I reworked the PR around the dogfood repro path from #12052. Locally, with the dogfood
I removed the helper-only test coverage because that was basically redundant and was mainly for proving the approach not the actual repro. I havent made a commit for it because i am still looking for a generic approach as you asked so it doesnt effect other plugins. |
|
Please use the exact same dogfood test case that I added here: #12052 We have proof that it reproduces the problem with the other PR, and we'll merge it as part of the fix to prevent any possible regression in the future. For the e2e test failures, it's an unrelated problem: just rebase your PR against main. |
|
I tested the exact #12052 dogfood repro in a clean sibling worktree instead of my earlier custom repro. With the default Rspack config, After I changed only: rspackBundler: false,
rspackPersistentCache: false,cleaned website/.docusaurus, website/build, and cache dirs, and the same command passed. So yeah, this looks Rspack-specific. I’ll rebase against main and update the PR around the exact #12052 dogfood test case, not the helper-only test. |
452c4e4 to
044b0a5
Compare
|
Updated the PR after rebasing against The diff now uses the exact #12052 dogfood repro files, removes the helper-only test, and keeps the fix scoped to the docs plugin while using a small shared metadata source resolver. Local validation confirms:
No Rspack config, website config, package, or lockfile changes are included. |
|
If this is a Rspack bug, then the fix must be reported with a minimal repro to the Rspack team I don't want to implement a weird workaround for a bug in a dependency, to solve a fancy edge case (who uses zero-width spaces in filenames 🤷♂️ ) |
|
Well honestly this issue seemed weird to me from the start , i picked it because had a feeling that there is a lot more to this than something that's being reported but i am still happy with the findings we got it at least gives a ground to work on future if need . Not to mention it also got super complicated I should have talked to you to understand if the PR is needed. I will keep that in mind for future 😄 |
Summary
Fixes #12005.
This PR resolves a docs metadata path mismatch for docs paths containing
U+200B ZERO WIDTH SPACE, using the exact dogfood repro from #12052.Changes
@docusaurus/utils.Validation
Ran:
Manual fail/pass check:
yarn build:website:fastfails with missing metadata JSON importsyarn build:website:fastpassesNo
rspackBundler,rspackPersistentCache, website config, package, or lockfile changes are included.