fix: babel config lookup #364
Conversation
I've added find-up as a dependency for finding the babel config file. The babel config files to look for have been taken from @babel/core's sourcecode Finally, as babel-loader itself will by default not look outside of the cwd for the config file, we don't want to have false positives for any babel config files that exist outside of the cwd Closes netlify#200, netlify#188
Use find-up matcher function to automatically stop looking for babel config file once cwd is reached, thus increasing performance.
Robin-Hoodie
force-pushed
the
fix/babel-config-lookup
branch
from
c001f06 to
b00e640
Compare
There was a problem hiding this comment.
Hi @Robin-Hoodie and sorry for the late reply.
Thank you again for pushing this fix. I think I wasn't able to explain myself before when I mentioned monorepo support. I've added a few commits to cleanup the tests and a test case describing what I had in mind:
https://github.com/netlify/netlify-lambda/pull/364/files#diff-dd24f19d79300a10592927150e05758644fc84a1ae5c7b7844ae0909416738d2R153
The scenario we'd like to support is if someones has a babel config file in the root of a monorepo, but runs netlify-lambda from the a directory of a specific package (hence the cwd is the directory of that package).
Please let me know what you think of the changes I made and if my explanation is clear enough.
| function webpackConfig(dir, { userWebpackConfig, useBabelrc } = {}) { | ||
| function webpackConfig( | ||
| dir, | ||
| { userWebpackConfig, useBabelrc, cwd = process.cwd() } = {}, |
There was a problem hiding this comment.
cwd argument is only here so we can test the monorepo setup
|
Ok, so after reading https://babeljs.io/docs/en/config-files#file-relative-configuration and https://babeljs.io/docs/en/config-files#monorepos it might be better to use |
That would definitely be the easiest and it's what I tried to do first. However, I don't know if The logic for looking up a babel config file does not seem to be exposed as far as I can tell I might well be wrong in my assumptions here though |
|
I went over the commits you've added just now. Added 2 comments but great additions overall! |
New PR from different branch which closes #359 .
In comparison to #359 the following has been added:
find-up's matcher function and improve performanceCloses #200, #188