-
Notifications
You must be signed in to change notification settings - Fork 245
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
fix(pacmak): TypeError when operating with Worker threads #2550
Conversation
This works around isaacs/node-graceful-fs#204, where `graceful-fs` cannot be loaded from within `Worker` threads. Since the language generators are loaded in worker threads whenever they are available, they cannot use `fs-extra` which itself uses `graceful-fs`. This is hopefully a temporary measure, until isaacs/node-graceful-fs#205 is merged & released.
@@ -46,8 +46,16 @@ export class Golang extends Target { | |||
|
|||
// delete local.go.mod and local.go.sum from the output directory so it doesn't get published | |||
const localGoSum = `${path.basename(localGoMod, '.mod')}.sum`; | |||
await fs.remove(path.join(pkgDir, localGoMod)); | |||
await fs.remove(path.join(pkgDir, localGoSum)); | |||
await fs.unlink(path.join(pkgDir, localGoMod)).catch((err) => |
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.
Please extract to a helper function
visit(dep), | ||
), | ||
), | ||
).then(() => void undefined); |
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.
Why not use await?
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.
Oh yeah you're right :P I thought I had... uh... a reason. but not.
const lines = (await fs.readFile(goMod, 'utf-8')).split('\n'); | ||
const lines = await fs | ||
.readFile(goMod, 'utf8') | ||
.then((text) => text.split('\n')); |
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.
Does this handle errors properly?
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.
Yep - .then
has no catch
handler, so it'd simply use the original rejection.
await unlink(path.join(pkgDir, localGoMod)); | ||
await unlink(path.join(pkgDir, localGoSum)); | ||
|
||
async function unlink(file: string): Promise<void> { |
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.
move outside
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Merging (with squash)... |
)" This reverts commit 5822e48.
This works around isaacs/node-graceful-fs#204, where
graceful-fs
cannot be loaded from within
Worker
threads. Since the languagegenerators are loaded in worker threads whenever they are available,
they cannot use
fs-extra
which itself usesgraceful-fs
.This is hopefully a temporary measure, until isaacs/node-graceful-fs#205
is merged & released.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.