refactor: simplify shimExecFile by removing type casts and promisify#880
Closed
jeremyruppel wants to merge 4 commits intofix/windows-cross-platform-compatfrom
Closed
refactor: simplify shimExecFile by removing type casts and promisify#880jeremyruppel wants to merge 4 commits intofix/windows-cross-platform-compatfrom
jeremyruppel wants to merge 4 commits intofix/windows-cross-platform-compatfrom
Conversation
…t electron path Unix-style `VAR=value` syntax in npm scripts doesn't work on Windows. Added cross-env to build:production, test, test:extension, and test:webview scripts. Changed vitest.nodeExecutable to point to the actual electron binary instead of the .bin shim, which doesn't resolve on Windows (only electron.cmd exists there).
- speedtest tests: use process.execPath instead of .cmd wrapper since execFile cannot spawn .cmd files on Windows (EINVAL) - sshProcess tests: use path.join for expected log paths so separators match the OS (backslashes on Windows, forward slashes on Unix) - Build integration tests on project build.
…handling Use Reflect.apply and a generic type parameter instead of `as unknown as` chains. Replace promisify usage in tests with plain Promise callbacks, which eliminates the need for the promisify custom symbol wrapping entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jeremyruppel
force-pushed
the
jeremyruppel/simplify-execfile-shim
branch
from
1adad24 to
f45cc1c
Compare
EhabY
force-pushed
the
fix/windows-cross-platform-compat
branch
from
0121302 to
12b454f
Compare
Collaborator
|
Ah actually the reflect usage seems much cleaner than casting |
EhabY
force-pushed
the
fix/windows-cross-platform-compat
branch
from
252a255 to
d57a977
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed
promisifywas adding some complexity and making strict typing difficult in this test. This usesReflect.applyand a generic type parameter instead ofas unknown aschains. Also replacespromisifyusage in tests with plain Promise callbacks, which eliminates the need for thepromisifycustom symbol wrapping entirely.