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

feat(v2): allow custom output directory for build #2417

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

ZachJW34
Copy link
Contributor

@ZachJW34 ZachJW34 commented Mar 16, 2020

Motivation

Resolves #2410

Docusaurus 2 added the ability to specify a siteDir, so it would be nice if the output directory of the build could be changed as well. I have been working in an nx workspace, and all builds end up in dist at the root with my docs website at apps/website. Currently, there is no way to place the build anywhere except for inside the siteDir.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I tested all these changes by running npx @docusaurus/init@next init to create a new repo. I built the packages and copied them over to the new repo.

  1. Ran yarn build to verify normal behavior

  2. Ran yarn build --outDir build/nested to verify custom output directory

  3. Created a nested version by copying all docusaurus related content into a nested folder called nested

  4. Ran yarn build nested to verify normal behavior

  5. Ran yarn build nested --outDir to verify custom output directory

  6. Created a github repo to test deploy.

  7. Tested the commands above with deploy (Nested project has some tweaks to the docs so as to ensure I could see the difference).

image

image

Related PRs

None

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 16, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Mar 16, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 18e92ec

https://deploy-preview-2417--docusaurus-2.netlify.com

@yangshun
Copy link
Contributor

Thanks for the PR. Did you see #2410 before creating this?

@ZachJW34
Copy link
Contributor Author

I didn't! I started working on this a few days ago and didn't check the issues. I saw this issue from a while back and thought I'd take a crack at it.

Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little improvements.

export async function deploy(siteDir: string): Promise<void> {
export async function deploy(
siteDir: string,
cliOptions: Partial<BuildCLIOptions>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cliOptions: Partial<BuildCLIOptions>,
cliOptions: Partial<BuildCLIOptions> = {},

@@ -59,6 +59,7 @@ Compiles your site for production.
| Options | Default | Description |
| --- | --- | --- |
| `--bundle-analyzer` | | Analyze your bundle with [bundle analyzer](https://github.com/webpack-contrib/webpack-bundle-analyzer) |
| `--outDir` | build | The full path for the new output directory, relative to the current workspace. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `--outDir` | build | The full path for the new output directory, relative to the current workspace. |
| `--outDir` | `build` | The full path for the new output directory, relative to the current workspace. |


| Options | Default | Description |
| --- | --- | --- |
| `--outDir` | build | The full path for the new output directory, relative to the current workspace. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `--outDir` | build | The full path for the new output directory, relative to the current workspace. |
| `--outDir` | `build` | The full path for the new output directory, relative to the current workspace. |

.action((siteDir = '.', {bundleAnalyzer}) => {
.option(
'--outDir <outDir>',
'The full path for the new output directory, relative to the current workspace.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'The full path for the new output directory, relative to the current workspace.',
'The full path for the new output directory, relative to the current workspace (default = build).',

wrapCommand(deploy)(path.resolve(siteDir));
.option(
'--outDir <outDir>',
'The full path for the new output directory, relative to the current workspace.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'The full path for the new output directory, relative to the current workspace.',
'The full path for the new output directory, relative to the current workspace (default = build).',

@ZachJW34
Copy link
Contributor Author

Updated!

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. I was thinking of just adding it to docusaurus.config.js. Was there any particular reasoning to add this option to the CLI command?

@ZachJW34
Copy link
Contributor Author

@yangshun Partly due to the talk in this issue. That was back in V1 so thoughts might have changed on that. Also, keeping the siteDir and outDir options next to each other made sense to me and having both the paths relative to some workspace root.

@yangshun
Copy link
Contributor

@lex111 what do you think? Should it be in the config or as a CLI option? Next.js puts it in the config while Gatsby added it then reverted it and now still don't have it (gatsbyjs/gatsby#1878).

@yangshun yangshun changed the title feat(v2) add custom output directory to build feat(v2): allow custom output directory for build Mar 18, 2020
@lex111
Copy link
Contributor

lex111 commented Mar 18, 2020 via email

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry can we do --out-dir instead to have consistent naming with our other CLI options?

Lemme know if you're not available to do it, I should be able to take it over and do the rename.

@ZachJW34
Copy link
Contributor Author

@yangshun Updated!

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@yangshun yangshun merged commit ce45413 into facebook:master Mar 19, 2020
@yangshun yangshun added the pr: new feature This PR adds a new API or behavior. label Mar 21, 2020
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: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support custom build path
5 participants