-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
feat(v2): add --config option to CLI #4308
Conversation
[V1] Deploy preview success Built with commit 9d67ddc |
Deploy preview for docusaurus-2 ready! Built with commit 9d67ddc |
Size Change: 0 B Total Size: 532 kB ℹ️ View Unchanged
|
f7aa8ee
to
76da25d
Compare
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4308--docusaurus-2.netlify.app/classic/ |
15b50dc
to
58d6fba
Compare
@slorber if u can review that'd be great! Thanks! |
Thanks, will take a deeper look tomorrow but that looks nice. Just wondering if having 2 new cli options for a quite niche use-case isn't going to clutter a bit our API? I don't have strong opinions, just wondering what's the best public API choice for this. |
yeah that works too I'm fine w/ either option. LMK and I'll change it :) |
Note: we could basically have a need to allow customizing all these constants: packages/docusaurus/src/constants.ts
At least it has been requested already the ability to customize My point is that I would find it odd if we would end up with many start/build options, most of them are barely used by anyone. |
Although the bigger question would be, is |
Honnestly I don't know too much the impact of changing And we could later leverage some kind of image resizing or Webpack 5 build caching inside it so we should be able to read from the |
ok gotcha. Yeah I don't really mind 1 way or the other as long as there's a way to customize it (env var, param, config file...) |
So, I see a few use-cases where --config can be useful as a cli option. For Going to edit a bit your PR and merge it asap, thanks |
@longlho have you tested this with I'm trying this and it fails:
This is somehow expected as this tmp folder does not have any node_modules to read, and the generated files contain require/import calls to import React. |
Note I tested to revert my local changes and it does not work either.
|
hmm gotcha I'll take a look. What's the server bundling btw? |
I don't mind merging a way to customize the .docusaurus folder, but would this actually solve your usecase if you can't use
What do you mean? Webpack? |
@slorber yes subfolder of the monorepo is what I'm looking for. For context bazel creates a sandbox for every build and symlink dependencies in, including By server bundling I mean I don't know what the server component of docusaurus is since I only use the client bundle and throw it on GH pages. |
So, just pushed my update. As a summary, this works:
But this won't work, and afaik it's not a problem for your use-case:
|
Motivation
We're trying to get
docusaurus
to play nicer with bazel, which relies heavily on explicit IO. Currentlydocusaurus
makes a lot of assumptions on folder structure based onsiteDir
(e.g.docusaurus
temp folder &docusaurus.config.js
). The new CLI options allow user to control:--config
controls the path todocusaurus.config.js
.--generated-files-dir
controls the path to.docusaurus
folder which is used for deployment.tmpdir
. This prevents issue where generated files dir is read-only (inbazel
scenario).Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Upstream bazel setup: https://github.com/formatjs/formatjs/blob/main/website/BUILD#L50
Related PRs