graphql-let icon indicating copy to clipboard operation
graphql-let copied to clipboard

Add `cwd` option to fix loader + jestTransformer when using moved cache + config options

Open comp615 opened this issue 2 years ago • 8 comments

Addresses #605

Adds a new cwd option to the config and pipes it through path resolutions. There's a number of folder and base folders potentially involved in the process of running graphql-let: process dir, config dir, and other dirs.

In cases where the config file does not reside in the jest root directory. This is necessary because the config file can then give the cache location, and it will be resolved relative to its own location (not that of the jest config). Additionally, documents CANNOT reside outside the root directory. Since we store them in the cache dir as a literal directory, relative paths escaping that store the cache files in non-sensical places, additionally, this would imply we could have folder conflicts. This change helps resolve that ambiguity.

Put differently, now by default all paths will resolve relative to the config dir itself. But if you have the config dir in a sub-directory, this allows you to "reset" the base resolution directory opening up a wider variety of configuration options regardless of where you run jest from, store your config, etc.

Changes:

  • Adds new cwd option
  • Adds documentation around the new option
  • Returns final cwd from processed config
  • Updates execContext to return that config.cwd value which should update all resolutions
  • Updates jestTransformer to use this value (instead of jest rootDir once we resolve the config)
  • Updates loader to do the same
  • Adds jestTransformer test
  • Updates loader test to fix it not actually using the temp dir and to exercise the new cwd option

comp615 avatar May 05 '22 22:05 comp615

Looks good, thank you @comp615! I'm ready to merge and publish once you fix the build failure, seemingly of a simple prettier error. Thanks!

piglovesyou avatar May 06 '22 01:05 piglovesyou

Looks good, thank you @comp615! I'm ready to merge and publish once you fix the build failure, seemingly of a simple prettier error. Thanks!

Thanks, will update, would also like to take a look at the test and see if I can add a case

comp615 avatar May 06 '22 01:05 comp615

All set now I think, wrote a new test!

comp615 avatar May 09 '22 14:05 comp615

@piglovesyou wanted to flag this has been fixed and added another small PR for some security fixes. Thanks! We're enjoying it so far.

comp615 avatar Jun 24 '22 15:06 comp615

NOTE: This same technique might need to be applied in more places like https://github.com/piglovesyou/graphql-let/blob/main/src/loader.ts, we continue to encounter this issue in various ways

comp615 avatar Aug 31 '22 14:08 comp615

@piglovesyou Wanted to check in and see if you were still around! I've opened a few more PRs and would love to get them in (along with some of the dependabot stuff). If you are swamped, I'd also be happy to take a go at doing some maintainer work here and on NPM if you're willing. Happy to setup time to talk more if that's useful.

comp615 avatar Apr 07 '23 17:04 comp615

Have updated this with a more complete solution.

comp615 avatar Jun 14 '23 21:06 comp615

@piglovesyou saw you were doing some maintenance last week. Any chance of getting this one in? This is the most critical change in my mind of the ones open. We're currently running on a fork based off this branch in order to allow us to run a monorepo environment.

comp615 avatar Oct 23 '23 17:10 comp615