-
Notifications
You must be signed in to change notification settings - Fork 22
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 additon of extraneous fields #132
Conversation
Nicer tests. Also made some minor changes to where some logic is located. Still haven't tracked down the issue, we must be mocking something wrong.
|
||
workspaces ??= [] | ||
const globPromises = workspaces.map((w) => | ||
const globPromises = workspaces!.map((w) => |
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.
Yes, non-null assertations are bad, but I figured they fail fast, unlike ??=
.
Same thing with the mocks.
Certainly negligible, but it looks cleaner this way too.
56ecb96
to
25d203d
Compare
I gave up writing regression tests, but otherwise I'm happy with this. |
Also, if developing on node>=18, you can apply a patch and build with Patch
diff --git a/bin/typesync b/bin/typesync
index 95dbe55..e28f57c 100755
--- a/bin/typesync
+++ b/bin/typesync
@@ -1,4 +1,2 @@
#!/usr/bin/env node
-const { startCli } = require('../lib/cli')
-
-startCli()
+import('../dist/cli.mjs').then(({ startCli }) => startCli()) Build command: |
I’m out on vacation, I’ll check on these when I get back. 👍 |
...subManifests.map(async (p) => | ||
syncFile( | ||
p, | ||
await packageJSONService.readPackageFile(p), |
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.
Just curious, what did this change vs just reading the file from the file path in syncFile
?
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 honestly don't remember 🤷♂️
Thanks, released as 0.13.4! |
Haven't tracked down the source of the bug yet.