-
-
Notifications
You must be signed in to change notification settings - Fork 557
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(core): load forge.config.ts
with tsx
on supported Node versions
#3881
Open
erickzhao
wants to merge
10
commits into
main
Choose a base branch
from
tsx
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
0badb22
to
bce9a66
Compare
ts-node
with tsx
forge.config.ts
with tsx
whenever possible
forge.config.ts
with tsx
whenever possibleforge.config.ts
with tsx
on supported Node versions
forge.config.ts
with tsx
on supported Node versionsforge.config.ts
with tsx
on supported Node versions
forge.config.ts
with tsx
on supported Node versionsforge.config.ts
with tsx
on supported Node versions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #3872 Fixes #3676 Fixes #3609 Fixes #3780 Fixes #3671
Context
#2993 added support for arbitrary non-JavaScript configuration files using Gulp's
interpret
andrechoir
modules.Given a list of file extensions supported by
interpret
,rechoir
registers that extension's module loader with Node.js.For most use-cases, this would load the
forge.config.ts
config file withts-node
, although any supported Interpret module loader would work (e.g. CoffeeScript, YAML, etc.)Problem statement
#3872 reported that TypeScript configurations stopped working in Node 23.6, which is the first version where type stripping in Node.js is available unflagged.
Simply using a
forge.config.ts
file fails to load with Node v23.6 and above, which indicates some sort of failure in thets-node
/interpret
/rechoir
toolchain.Solution
ts-node
hasn't had a release in over a year and has known issues with ESM compatibility.tsx
has grown as a popular alternative to running TypeScript in Node. Note thatinterpret
andrechoir
currently don't supporttsx
as a module loader, either.tsx
comes with its ownregister()
API for both CJS and ESM, but requires at least Node v18.19 or v20.6 because it uses themodule.register
API under the hood.This leaves us with an incomplete support matrix between the
rechoir
andtsx
methods, where:rechoir
loader.tsx
loader.ts-node
in general has spotty ESM support.Thankfully, this means that all versions where
ts-node
is unsupported are supported bytsx
.I wrote a new
supportsModuleRegister
helper function incore-utils
that returns whether or not we can usemodule.register
(and thereforetsx
). This splits the code into two paths:Miscellaneous
tsx
also adds support forforge.config.mts
and potentially solves ESM-related configuration issues for us.FusesPlugin.slow.spec.ts
now symlinks to the local version of Electron Forge.chmod
to theinitLink
helper to make theelectron-forge
symlink executable. This is potentially due to a bug in Yarn Classic (Permission denied for package after using yarn link yarnpkg/yarn#8206).EBUSY
errors on Windows CI forWebpackTypeScriptTemplate.slow.spec.ts
that were previously fixed in the Vite template by @BlackHole1 (ref feat(plugin-vite): upgrade to vite@5 #3468 (comment)).startLogic
PluginBase function that we don't use as of feat: add preStart hook and port existing startLogic impls #3720 now fails because we're loading the config viatsx
and the function reference equality no longer applies. My hacky workaround is to compare the.toString()
value of those functions, but that might be unsatisfactory.Reviewing this PR
The core set of changes is in:
packages/api/core/src/util/forge-config.ts
: Forge config loading business logicpackages/utils/core-utils/src/supports-module-register.ts
: New utility to check if we can usetsx
to load the Forge configpackages/template/webpack-typescript/src/WebpackTypeScriptTemplate.ts
: Conditionally installsts-node
ifsupportsModuleRegister
is false.packages/template/vite-typescript/src/ViteTypeScriptTemplate.ts
: Conditionally installsts-node
ifsupportsModuleRegister
is false.The rest of the file changes follow from there!