Skip to content

feat: add ability to specify custom build output directory#14

Merged
brkalow merged 3 commits intohashicorp:mainfrom
jzxhuang:feat/custom-build-output-directory
Feb 18, 2022
Merged

feat: add ability to specify custom build output directory#14
brkalow merged 3 commits intohashicorp:mainfrom
jzxhuang:feat/custom-build-output-directory

Conversation

@jzxhuang
Copy link
Copy Markdown
Contributor

Adds the ability to specify a custom build output directory.

Notes:

  • Thanks for your great work on this! Looking to make this change to make it usuable in a Nx monorepo I work with, where the app is built to a path that looks like dist/apps/... (per Nx recommendations)
  • Tests are passing, but didn't add any new tests — I think this would require creating a completely new fixture with a different package.json. If you'd like, I can add a new test suite
  • Could probably pull BUILD_OUTPUT_DIRECTORY into a file named utils.js or something 🤷

@hashicorp-cla
Copy link
Copy Markdown

hashicorp-cla commented Jan 22, 2022

CLA assistant check
All committers have signed the CLA.

@jzxhuang
Copy link
Copy Markdown
Contributor Author

cc @jescalan thanks for creating this! Added a small PR here to support non-standard build directory.

and if you're reading this, I also [opened an issue]("license": "GPL-2.0",) to add a LICENSE file to the repo root. Cheers 😄

@jescalan
Copy link
Copy Markdown
Collaborator

jescalan commented Feb 1, 2022

Hey sorry for the slow response here -- if you could add a test for this, that would be really wonderful 🙏

@jzxhuang jzxhuang force-pushed the feat/custom-build-output-directory branch from f831078 to 9ec6626 Compare February 13, 2022 22:50
@jzxhuang
Copy link
Copy Markdown
Contributor Author

@jescalan no worries, added a test! I had to rejig the fixtures setup to do so, making each "fixture" its own folder and updated the test script to iterate through them accordingly.

The new fixture custom-output-directory is a copy of the existing fixture, except that we output the next build to /dist in next.config.js and specify this accordingly in nextBundleAnalysis of package.json

@jescalan
Copy link
Copy Markdown
Collaborator

@jzxhuang this looks really great, thank you so much for this huge contribution!

@brkalow would you be able to take a quick look at this, maybe run the tests locally and merge? I lost access since I don't work at hashicorp anymore 😭

Copy link
Copy Markdown
Contributor

@brkalow brkalow left a comment

Choose a reason for hiding this comment

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

So sorry we missed this, thanks for adding this it looks great!

@brkalow brkalow merged commit c5a1e07 into hashicorp:main Feb 18, 2022
@jzxhuang
Copy link
Copy Markdown
Contributor Author

thanks @brkalow! Would you also be able to:

ty 😇

@brkalow
Copy link
Copy Markdown
Contributor

brkalow commented Feb 23, 2022

Published, thanks for your patience! https://github.com/hashicorp/nextjs-bundle-analysis/releases/tag/0.4.0

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.

4 participants