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: Add book-specific config #26

Merged
merged 2 commits into from
Mar 10, 2022
Merged

feat: Add book-specific config #26

merged 2 commits into from
Mar 10, 2022

Conversation

wittjosiah
Copy link
Member

No description provided.

@@ -88,7 +88,7 @@ export const bookCommand: CommandModule<{}, BookCommandArgv> = {
const projectRoot = process.cwd();
const packageRoot = getPackageRoot();
const staticDir = join(packageRoot, 'src/book/ui/public');
const outdir = config?.outdir || './dist';
const outdir = config?.book?.outdir || './out-book';
Copy link
Member

Choose a reason for hiding this comment

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

I notied the conflict here, but don't like compound directory names. Can this be made to work with out/book?

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha, I had a feeling you were going to suggest this! I think this can definitely work but comes with a different tradeoff: either that will mean that the book output is included in the bundle (likely harmless but wasteful and messy) or we need to path the default output of the app as well. If we path the default output of the app then it will no longer match the default path of the dx ns deploy command and it will always need to be specified.

Given all this I think my suggestion would be:

  • default esbuild-server outdir is out
    • this would work the default dx ns deploy command
  • default esbuild-server book outdir is out/book
    • we'll need to specify the path for the deploy command for this no matter what it is since it can't match the default so that's fine
  • if you're building both, then you would need to change the outdir config to something like out/app and specify that for the deploy path as well
  • this keeps the default usecase simple but allows for flexibilty for our usecase where we're publishing the storybooks as well

@wittjosiah wittjosiah merged commit fe4e08d into main Mar 10, 2022
@wittjosiah wittjosiah deleted the wittjosiah/book-config branch March 10, 2022 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants