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

configurable output paths #12501

Closed
wants to merge 25 commits into from
Closed

Conversation

mathieudutour
Copy link
Contributor

@mathieudutour mathieudutour commented Mar 11, 2019

Description

Introduces command line options (also handles env variables) to specify the output directory locations (both .cache and public).

The two options are --cache-dir and --build-dir (with the respective process.env.GATSBY_CACHE_DIR and process.env.GATSBY_BUILD_DIR variables).

There shouldn't be any breaking changes if the different packages are updated at the same time but if not, some plugins are now depending on new APIs defined on the cache object to access the cache and public folders.

It also provides 2 webpack aliases to access both folders (see https://github.com/gatsbyjs/gatsby/pull/12501/files#diff-917ba78a52f29b1a1fe42be34d81fc83R16)

Related Issues

Fixes #1878, #5880

@mathieudutour mathieudutour requested a review from a team as a code owner March 11, 2019 22:46
@mathieudutour mathieudutour requested a review from a team March 11, 2019 22:46
@mathieudutour mathieudutour reopened this Mar 12, 2019
@mathieudutour mathieudutour force-pushed the f/configurable-output-paths branch from e5b1320 to 06ce2f5 Compare March 12, 2019 12:07
packages/gatsby-cli/src/create-cli.js Outdated Show resolved Hide resolved
packages/babel-preset-gatsby/src/index.js Outdated Show resolved Hide resolved
@muescha
Copy link
Contributor

muescha commented Mar 12, 2019

are the docs should also include the new options?

@mathieudutour
Copy link
Contributor Author

probably yes, and there should also be some tests. I'm waiting for more feedback if that's something we want (or the right direction) before putting more time into it :)

@mathieudutour
Copy link
Contributor Author

I'm confused as to why the ci/circleci: integration_tests_gatsby_pipeline test fails, it works fine locally

@mathieudutour mathieudutour force-pushed the f/configurable-output-paths branch from d9ea11a to 0e24bd8 Compare March 12, 2019 15:30
@mathieudutour mathieudutour force-pushed the f/configurable-output-paths branch from 5b0ad09 to 09f6aa5 Compare March 12, 2019 23:13
@mathieudutour mathieudutour added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Mar 14, 2019
@DSchau
Copy link
Contributor

DSchau commented Mar 15, 2019

@mathieudutour and I chatted about this a little in Discord.

In the interest of explaining the why of where we're coming from, I think it's first helpful to depict the problem that this is solving.

@mathieudutour is working on a super-slick Lambda/CI build tool that will clone/fetch repos on changes (to the /tmp directory, which is writable in Lambda), build out those repos using a root-level gatsby instance, and deploy the build content to s3.

gatsby (the package) is embedded within a CLI tool, e.g. we have the following directory structure for the Lambda:

tmp/
  repos/
    some-repo/
      some-content.md
node_modules/
  @org/some-cli-tool
  some-package-that-has-gatsby/
    gatsby-config.js
  gatsby # installed by @org/some-cli-tool

This is all fine and dandy, except that Lambda only allows /tmp to be writable. So when @org/some-cli-tool build is run, gatsby build is invoked from the root of the lambda, therefore trying and failing to create the folders .cache and public.

This is a valid, novel use-case for using Gatsby CLI programatically from a lambda. It would be ideal if we could support this use case, and others which require custom build directories and cache directories. Also, most other tools I just spot checked, e.g. Next.js, Vuepress, etc. seem to support a custom build directory.

That being said--I don't think the complexity of this PR is the best solution to this problem at this time.

Why

Complexity

Given the condition that Gatsby should be able to be configured with a custom build and cache path (which I agree with)--I don't necessarily agree that the solution is one-off slightly misfit APIs, e.g. cache.publicPath() (also this seems very strange to be used for e.g. joining public with some file path). This is not really discoverable, and it's hard to maintain and control this via plugins--particularly those that we do not maintain. This makes this change extremely brittle.

If we were to introduce something like this--it should be handled automatically, whenever possible. Making this opt-in with a non-intuitive API in some sense makes this incredibly easy for some random plugin to break this workflow entirely.

Additionally - as noted, this is a breaking change (for any updated plugin on an older version of Gatsby) that can be mitigated with some guards, but this introduces more complexity to a PR that is already fairly complex and is touching many disparate pieces of functionality.

This complexity feeds into my next point:

General vs. niche

This PR introduces a complex, slightly brittle change to Gatsby core for a use case well outside the norm. If we merge this PR, we have to maintain this complexity for all use cases even though it seems apparent that this use case is very much more niche than generally applicable. Could this complexity be better managed at the implementer level (e.g. with tweaks to the lambda, deployment process, etc.) than in the Gatsby core code base? Could this change be better managed (for core maintainers) if we introduce it in a major release with the option for breaking changes?

I think it's pretty clear in this case that yeah, the complexity and niche use case makes this better served by implementing some other suggestions, specifically:

Suggestions

Trying to rank these in order of complexity/cost/timelines:

  1. Find a way to run Gatsby outside the root of the lambda, e.g. in /tmp/repos/some-repo
    • e.g. if those repos depended on the CLI tool, that could work (?)
    • You mentioned this increases cost (?)
  2. Run the build on EC2
    • Not sure how far along the lambda is, but I'd imagine this control will possibly be needed anyways (e.g. to get a perfectly configured Docker image, system, etc.)
  3. Tweak this general idea to bake this more into core and make it automatic
    • To do this correctly, we'll likely need to introduce some breaking changes, and we'd probably do this in something like Gatsby 3.0.0 to handle these

I'm well aware this is probably not a decision that you'll be super fond of, so want to make it clear that:

  1. We'd like this feature too, just want to roll it out in the most seamless manner rather than in a way that seems prone to breakage
    • I'll re-open the custom build directory issue, so we can continue to track on this and prioritize
  2. We'll happily pair with you on this, just may require a bit more of input from the core team so we can decide how we'd like to implement this (perhaps we discuss more in Configurable output folder #1878?)
  3. If you'd like some Gatsby swag for your efforts, please do let me know and I'll create a code--you deserve it!

Once-more, if I haven't made it crystal clear, thank you for this PR. We appreciate it--very much!

@DSchau DSchau closed this Mar 15, 2019
@mathieudutour
Copy link
Contributor Author

mathieudutour commented Mar 15, 2019

Thanks for the detailed answer.

one-off slightly misfit APIs, e.g. cache.publicPath()

Totally agree that this is weird and should probably go somewhere else. It was simply the easiest, and I'd happily accept suggestions. But that's really a detail.

This is not really discoverable, and it's hard to maintain and control this via plugins--particularly those that we do not maintain. This makes this change extremely brittle.

I don't think it will. It won't change anything for people until one user tries to use a custom path. Then it's about fixing what is effectively a bug in the plugin. The ecosystem can slowly be fixed without breakage for existing users. They shouldn't rely on hardcoded path anyway. That's what I've done preemptively for gatsby-mdx for example: ChristopherBiscardi/gatsby-mdx#310.

That's true that it's not really discoverable but that's true for pretty much all the plugin APIs right now.

it should be handled automatically, whenever possible.

That's interesting, I couldn't find any way to do that. Because plugins are just node script, they can do anything they want, the paths can be (and often are) dynamically generated so it'd be hard to force anything on them.

Could this complexity be better managed at the implementer level (e.g. with tweaks to the lambda, deployment process, etc.) than in the Gatsby core code base?

That's the thing right, all of those changes feel like workarounds for something that should be in core, and you seem to agree. All other frameworks support that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants