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
support all config file extensions (.js,.mjs,...) #3204
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3204 +/- ##
==========================================
+ Coverage 90.63% 90.63% +<.01%
==========================================
Files 167 167
Lines 5905 5906 +1
Branches 1792 1792
==========================================
+ Hits 5352 5353 +1
Misses 336 336
Partials 217 217
Continue to review full report at Codecov.
|
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.
Test and doc changes look good, but I wonder if we might solve similar issues once and for all by allowing every extension, see my suggestion.
cli/run/loadConfigFile.ts
Outdated
defaultLoader(module, filename); | ||
} | ||
}; | ||
const extensions = ['.js','.mjs']; |
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.
This will work nicely for '.js' and '.mjs' but not other extensions, and is slightly complicated. How about this: As we KNOW the name of the config file, how about using path.extname
to extract the extension of the config file and just modify the loader for this extension?
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.
makes perfect sense
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.
Thanks a lot, this looks really nice now!
I am trying to use this and I am calling like...
but I get...
Notice it seems to be using the cjs loader still. |
looks like this does work (lerna and my node_modules is in root for dev deps)
Is there a way to import without needing the relative path? |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers: #3189
Description
support naming the config file *.mjs