-
Notifications
You must be signed in to change notification settings - Fork 922
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
chore: avoid require to load package.json #1890
Conversation
12957cc
to
81cddd5
Compare
f5c8fe5
to
d2608c1
Compare
d2608c1
to
71e7d12
Compare
PTAL The new function has 100 % test coverage and is implemented as resilient as possible. |
): { [key: string]: any } { | ||
let pkgContent; | ||
try { | ||
pkgContent = fs.readFileSync(filepath, "utf8"); |
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 don't like that we have sync FS access, but that's the case today as well, we can address that in a future change, it's internal anyway.
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.
As long as we use express we will have issues because express can not have async handlers.
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.
got it, good to know.
🎉 This PR is included in version 13.0.0-beta.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
We avoid with this PR to load some arbitrary file via require.
Require could result in issues in esm environments.