-
Notifications
You must be signed in to change notification settings - Fork 39
feat: add resolveConfigPath
to return resolved config file path
#262
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
base: main
Are you sure you want to change the base?
Conversation
Resolve configuration file path: | ||
|
||
```js | ||
// Resolve the path to the configuration file without loading it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If layers are used resolveConfigPath > resolveSources
might also download layer which is not mentioned (also loading dotenv which is a side-effect) ~> #262 (comment)
|
||
const res = await resolveSources(".", options); | ||
|
||
return res.configFile; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we could make function more generic like resolePaths
or resolveConfig
returning an object either full res or a subset, cwd
and configFile
are particulary useful. If we also resolve rc config, it can be useful for automation scripts to know where to edit/append it. We also have information about layers alrady.
|
||
// Load dotenv | ||
if (options.dotenv) { | ||
await setupDotenv({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this? It might be helpful for cloning private layers i guess during resoluition but both are hidden side-effects specially if users pass same configuration they expect to pass to resolver and assume it will only try to resolve (not download or filling global env)
I am feeling we could make it opt-in with something like { sideEffect: true }
or { loadLayers: true }
or avoid side-effects from this function.
nuxt/test-utils#1381 (comment)
this is a first stab at extracting logic for
resolveConfigPath
. I've left all behaviour untouched and still running in the same order, just pulled out a couple of routines which seemed relevant for resolving the source file - which means that there's probably scope for simplifying thisI've also switched to explicit exports for
index.ts
but of course that could be reverted