Conversation
facebook-github-bot
added
the
CLA Signed
|
@Huxpro has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@slorber has updated the pull request. You must reimport the pull request before landing. |
|
ready for review, let me know if anything does not work |
Huxpro
left a comment
Huxpro
left a comment
There was a problem hiding this comment.
Thanks for doing this! Left some questions so I can better understand these changes :)
| 1. For Android, this happened in `hermes/android/build.gradle` | ||
| 2. For Apple platforms, this happend in `hermes/utils/build-apple-framework.sh` | ||
| 3. For Emscripten, you can find an example from the `test-emscripte` job from `hermes/.circleci/config.yml`. Also see more details at [Building with Emscripten](../emscripten) | ||
| 3. For Emscripten, you can find an example from the `test-emscripte` job from `hermes/.circleci/config.yml`. Also see more details at [Building with Emscripten](./Emscripten.md) |
There was a problem hiding this comment.
I'm guessing this is better because it works on both github.com and hermes website (with Docusaurus replacing the link to the correct url)?
There was a problem hiding this comment.
Docusaurus has a broken links checker now and this URL was reported as broken.
In general, I think it's a bad idea to link using relative URL paths, because it makes the link sensitive to your host provider adding an extra slash at the end of an URL or not (it is not something very std and lead to a ton of reported problems in all Jamstack frameworks) (our issue: facebook/docusaurus#3372)
Relative file paths are resolved to absolute URLs by Docusaurus at build time, so this is not sensitive to the host adding a trailing slash.
There was a problem hiding this comment.
(and yes, it also works on Github 👍 )
There was a problem hiding this comment.
Oh thanks for providing that context. I certainly fought with trailing slash appended by github pages before: https://twitter.com/Huxpro/status/798816417097224193
| "dependencies": { | ||
| "@docusaurus/core": "^2.0.0-alpha.69", | ||
| "@docusaurus/preset-classic": "^2.0.0-alpha.69", | ||
| "@docusaurus/core": "2.0.0-alpha.75", |
There was a problem hiding this comment.
This seems to be really strict. Is taking minor release w/ ^ bad?
There was a problem hiding this comment.
Not bad, it's just that after my tests using canary 2.0.0-alpha.hash somehow using ^ did not make yarn download alpha75 and the lockfile stayed on the canary version (probably because my version name is bad)
I can revert to ^ if you want but keep in mind we force users to have all core package versions aligned.
We'll add a "docusaurus upgrade" cli anyway so it does not matter much, up to you
There was a problem hiding this comment.
Thanks. No need to revert. We can always change that if we really need to.
|
@Huxpro has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
👌 Thanks, happy to have one more site on webpack 5 🤗 |
3 participants
Use an unreleased version of Docusaurus to test the webpack5 support works for this site, detect potential problems, and perf improvements.
See also facebook/docusaurus#4089