-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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: allow links to JSON in .md files to be transformed as asset links #4827
Conversation
closes facebook#3561 It seems to be a common problem that many people are having see: https://stackoverflow.com/questions/65307533/link-to-static-json-file Co-authored-by: Anthony McCaigue <anthony@nquiringminds.com> Co-authored-by: Alois Klink <alois@nquiringminds.com>
✔️ [V2] 🔨 Explore the source changes: 091d97b 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61eccc46c15dcf0007493929 😎 Browse the preview: https://deploy-preview-4827--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4827--docusaurus-2.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing.
I'm not sure to understand what the problem is in the first place, and also why your solution is a good one.
Please give me context so that I understand better.
Also please add an example in one of our dogfooding pages, I suggest to add something in src/pages/markdown-tests.md
, so that we are sure it works and that it keeps working over time.
I tried the following and it didn't seem to work
## Link to JSON file
- [manifest md link](@site/static/manifest.json)
- <a href={require("@site/static/manifest.json").default}>manifest jsx link</a>
@@ -122,6 +123,7 @@ export function getBabelOptions({ | |||
babelrc: false, | |||
configFile: babelOptions, | |||
caller: {name: isServer ? 'server' : 'client'}, | |||
exclude: new RegExp(`"/${STATIC_DIR_NAME}/"`), // exclude static dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this problem better solved using babel config? Shouldn't this be solved at the webpack level? (we have a hidden feature to allow using alternate js loaders esbuild, will it work in such case?)
is it normal to have " around the path?
won't this kick in for any /static/ folder (what if users have src/pages/static/xyz for example?)
What about the else case (babelOptions object?)
The right fix should be: in Markdown links, JSON extensions should be converted to Edit. it seems JSON links are already converted to file-loader calls... require('!file-loader?name=assets/files/[name]-[hash].[ext]!../../static/manifest.json').default I have no idea why it's still attempted to be parsed |
I'm taking this over. However this is much harder as it seems, because Webpack 4+ automatically loads JSON as data even with file-loader. (webpack-contrib/file-loader#259, webpack/webpack#6586) We are migrating to asset modules (#4708) but probably not in a while due to blocking issues about plugin compatibility. I'll see what are the possibilities to solve this. currently build is failing, which is expected. |
Fair. I doubt if this is used a lot anyways, and for the rest there's no behavior difference. Will merge it since it's just temporary |
closes #3561
It seems to be a common problem that many people are having see:
https://stackoverflow.com/questions/65307533/link-to-static-json-file
Co-authored-by: Anthony McCaigue anthony@nquiringminds.com
Co-authored-by: Alois Klink alois@nquiringminds.com
Motivation
There is a common problem with links to JSON files see:
https://stackoverflow.com/questions/65307533/link-to-static-json-file
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)