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
gh-75 fixing babel cache issue #288
Conversation
This will duplicate functionality so closes #290 |
Codecov Report
@@ Coverage Diff @@
## main #288 +/- ##
==========================================
+ Coverage 98.21% 98.97% +0.76%
==========================================
Files 1 1
Lines 56 98 +42
==========================================
+ Hits 55 97 +42
Misses 1 1
Continue to review full report at Codecov.
|
verbose: false, | ||
...options, | ||
} | ||
const babelMode = process.env[options.envName] || (process.env.BABEL_ENV && process.env.BABEL_ENV !== 'undefined' && process.env.BABEL_ENV !== 'development' && process.env.BABEL_ENV) || process.env.NODE_ENV || 'development' |
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.
You might want to wrap this in cache.using
, so that if the env changes without restarting Node.js the plugin is reinstantiated.
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.
thank you! I moved things around. I still don't know if it will reload properly in hot reload, but I don't think the environment mode can change without a js reload so I left that out. Hopefully it works in other tests when we release it in @next
!
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 am the second maintainer to this project so I'm not even sure about how all the babel features work
index.js
Outdated
console.log('dotenvMode', babelMode) | ||
} | ||
|
||
api.cache.using(() => { |
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.
The function passed to api.cache.using
should return a value that, when it changes, causes the plugin to be re-instantiated. Since this one always returns undefined
, the plugin will never be re-instantiated and it will always use the initial .env
file.
I suggest doing something like this:
function mtime(filePath) {
try {
return fs.statSync(filePath).mtimeMs
} catch {
return null
}
}
api.cache.using(() => mtime(options.path))
api.cache.using(() => mtime(localFilePath))
api.cache.using(() => mtime(modeFilePath))
api.cache.using(() => mtime(modeLocalFilePath))
let env
if (options.safe) {
const parsed = parseDotenvFile(options.path, options.verbose)
const localParsed = parseDotenvFile(localFilePath, options.verbose)
const modeParsed = parseDotenvFile(modeFilePath, options.verbose)
const modeLocalParsed = parseDotenvFile(modeLocalFilePath, options.verbose)
env = Object.assign(Object.assign(Object.assign(parsed, modeParsed), localParsed), modeLocalParsed)
} else {
dotenv.config({
path: modeLocalFilePath,
silent: true,
})
dotenv.config({
path: modeFilePath,
silent: true,
})
dotenv.config({
path: localFilePath,
silent: true,
})
dotenv.config({
path: options.path,
})
env = process.env
}
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.
so when it is re-initialized, pre()
is rerun?
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.
so I found that this did not work in tests unfortunately. do I also need to use addExternalDependency
?
9ecc087
to
5bd695a
Compare
Link to issue
fixes #75
Description of changes being made