Skip to content
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
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"clean": "rimraf dist && lerna exec -- rimraf dist tsconfig.tsbuildinfo",
"build": "tsc -b packages",
"build:watch": "yarn build --watch",
"postbuild": "ts-node tools/test-dist",
"postbuild": "tsx tools/test-dist.ts",
"docs": "yarn build && npx typedoc",
"lerna:publish": "lerna publish --force-publish --conventional-commits --no-changelog --exact",
"lint:js": "prettier --check . && eslint . --cache",
Expand All @@ -24,8 +24,8 @@
"test": "xvfb-maybe vitest run --project fast --project slow",
"test:fast": "xvfb-maybe vitest run --project fast",
"test:slow": "xvfb-maybe vitest run --project slow",
"test:clear": "ts-node tools/test-clear",
"postinstall": "rimraf node_modules/.bin/*.ps1 && ts-node ./tools/gen-tsconfigs.ts && ts-node ./tools/gen-ts-glue.ts",
"test:clear": "tsx tools/test-clear.ts",
"postinstall": "rimraf node_modules/.bin/*.ps1 && tsx ./tools/gen-tsconfigs.ts && tsx ./tools/gen-ts-glue.ts",
"prepare": "husky install",
"preversion": "yarn build"
},
Expand Down Expand Up @@ -70,6 +70,7 @@
"semver": "^7.2.1",
"source-map-support": "^0.5.13",
"sudo-prompt": "^9.1.1",
"tsx": "^4.19.3",
"username": "^5.1.0",
"vite": "^5.0.12",
"webpack": "^5.69.1",
Expand Down Expand Up @@ -119,7 +120,6 @@
"prettier": "^2.4.0",
"rimraf": "^3.0.1",
"syncpack": "^11.2.1",
"ts-node": "^10.0.0",
"typedoc": "0.25.13",
"typescript": "~5.4.5",
"vitest": "^3.0.3",
Expand Down
1 change: 1 addition & 0 deletions packages/api/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"semver": "^7.2.1",
"source-map-support": "^0.5.13",
"sudo-prompt": "^9.1.1",
"tsx": "^4.19.3",
"username": "^5.1.0"
},
"engines": {
Expand Down
22 changes: 18 additions & 4 deletions packages/api/core/spec/fast/util/forge-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,24 @@ describe('findConfig', () => {
expect(conf.buildIdentifier).toEqual('yml');
});

it('should resolve the TS file exports of forge.config.ts if config.forge does not exist and the TS config exists', async () => {
const fixturePath = path.resolve(__dirname, '../../fixture/dummy_default_ts_conf');
const conf = await findConfig(fixturePath);
expect(conf.buildIdentifier).toEqual('typescript');
describe('TypeScript', () => {
it('should resolve forge.config.ts', async () => {
const fixturePath = path.resolve(__dirname, '../../fixture/dummy_default_ts_conf');
const conf = await findConfig(fixturePath);
expect(conf.buildIdentifier).toEqual('typescript');
});

it('should resolve forge.config.cts', async () => {
const fixturePath = path.resolve(__dirname, '../../fixture/dummy_default_cts_conf');
const conf = await findConfig(fixturePath);
expect(conf.buildIdentifier).toEqual('typescript-commonjs');
});

it('should resolve forge.config.mts', async () => {
const fixturePath = path.resolve(__dirname, '../../fixture/dummy_default_mts_conf');
const conf = await findConfig(fixturePath);
expect(conf.buildIdentifier).toEqual('typescript-esm');
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type { ForgeConfig } from '@electron-forge/shared-types';

const config: ForgeConfig = {
buildIdentifier: 'typescript-commonjs',
};

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "",
"productName": "",
"version": "1.0.0",
"description": "",
"main": "src/index.js",
"type": "commonjs",
"scripts": {
"start": "electron-forge start"
},
"keywords": [],
"author": "",
"license": "MIT",
"devDependencies": {
"@electron-forge/shared-types": "*",
"electron": "99.99.99"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type { ForgeConfig } from '@electron-forge/shared-types';

const config: ForgeConfig = {
buildIdentifier: 'typescript-esm',
};

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "",
"productName": "",
"version": "1.0.0",
"description": "",
"main": "src/index.js",
"type": "module",
"scripts": {
"start": "electron-forge start"
},
"keywords": [],
"author": "",
"license": "MIT",
"devDependencies": {
"@electron-forge/shared-types": "*",
"electron": "99.99.99"
}
}
2 changes: 2 additions & 0 deletions packages/api/core/src/api/init-scripts/init-link.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import fs from 'node:fs';
import path from 'node:path';

import { PMDetails, spawnPackageManager } from '@electron-forge/core-utils';
Expand Down Expand Up @@ -34,6 +35,7 @@ export async function initLink<T>(pm: PMDetails, dir: string, task?: ForgeListrT
});
}
}
await fs.promises.chmod(path.resolve(dir, 'node_modules', '.bin', 'electron-forge'), 0o755);
} else {
d('LINK_FORGE_DEPENDENCIES_ON_INIT is falsy. Skipping.');
}
Expand Down
45 changes: 37 additions & 8 deletions packages/api/core/src/util/forge-config.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import path from 'node:path';

import { supportsModuleRegister } from '@electron-forge/core-utils';
import { ForgeConfig, ResolvedForgeConfig } from '@electron-forge/shared-types';
import chalk from 'chalk';
import fs from 'fs-extra';
import * as interpret from 'interpret';
import interpret from 'interpret';
import { template } from 'lodash';
import * as rechoir from 'rechoir';
import rechoir from 'rechoir';

// eslint-disable-next-line n/no-missing-import
import { dynamicImportMaybe } from '../../helper/dynamic-import.js';
import { dynamicImportMaybe } from '../../helper/dynamic-import';

import { runMutatingHook } from './hook';
import PluginInterface from './plugin-interface';
import { readRawPackageJson } from './read-package-json';

/* eslint-disable @typescript-eslint/no-require-imports */
// TSX imports only work with Node16 resolution while we're still on CommonJS resolution in tsconfig.base.json
// However, all of Vite's entire TS types break when using CommonJS with Node16 resolution, it's more approachable
// to use `require` directly for the time being.
const tsxCJS = require('tsx/cjs/api');
const tsxESM = require('tsx/esm/api');
/* eslint-enable @typescript-eslint/no-require-imports */

const underscoreCase = (str: string) =>
str
.replace(/(.)([A-Z][a-z]+)/g, '$1_$2')
Expand Down Expand Up @@ -99,6 +108,9 @@ export function fromBuildIdentifier<T>(map: BuildIdentifierMap<T>): BuildIdentif
};
}

/**
* Checks specifically if the Forge config is a file path on disk.
*/
export async function forgeConfigIsValidFilePath(dir: string, forgeConfig: string | ForgeConfig): Promise<boolean> {
return typeof forgeConfig === 'string' && ((await fs.pathExists(path.resolve(dir, forgeConfig))) || fs.pathExists(path.resolve(dir, `${forgeConfig}.js`)));
}
Expand Down Expand Up @@ -130,10 +142,14 @@ export default async (dir: string): Promise<ResolvedForgeConfig> => {
}

if (!forgeConfig || typeof forgeConfig === 'string') {
for (const extension of ['.js', ...Object.keys(interpret.extensions)]) {
// interpret.extensions doesn't support `.mts` files
for (const extension of ['.js', '.mts', ...Object.keys(interpret.extensions)]) {
const pathToConfig = path.resolve(dir, `forge.config${extension}`);
if (await fs.pathExists(pathToConfig)) {
rechoir.prepare(interpret.extensions, pathToConfig, dir);
// Use rechoir to parse any alternative syntaxes (except for TypeScript when tsx register is supported)
if (!supportsModuleRegister(process.version) || !['.cts', '.mts', '.ts'].includes(extension)) {
rechoir.prepare(interpret.extensions, pathToConfig, dir);
}
forgeConfig = `forge.config${extension}`;
break;
}
Expand All @@ -143,14 +159,27 @@ export default async (dir: string): Promise<ResolvedForgeConfig> => {

if (await forgeConfigIsValidFilePath(dir, forgeConfig)) {
const forgeConfigPath = path.resolve(dir, forgeConfig as string);
let unregisterCJS, unregisterESM;
try {
// The loaded "config" could potentially be a static forge config, ESM module or async function
// Register tsx enhancements
unregisterCJS = tsxCJS.register();
unregisterESM = tsxESM.register();

// The loaded "config" could potentially be a static Forge config, ESM module, or async function
const loaded = (await dynamicImportMaybe(forgeConfigPath)) as MaybeESM<ForgeConfig | AsyncForgeConfigGenerator>;
const maybeForgeConfig = 'default' in loaded ? loaded.default : loaded;
forgeConfig = typeof maybeForgeConfig === 'function' ? await maybeForgeConfig() : maybeForgeConfig;
} catch (err) {
console.error(`Failed to load: ${forgeConfigPath}`);
console.error(chalk.red(`Failed to load config file at ${chalk.green(forgeConfigPath)}`));
console.error(chalk.red(err));
throw err;
} finally {
if (typeof unregisterCJS === 'function') {
unregisterCJS();
}
if (typeof unregisterESM === 'function') {
unregisterESM();
}
}
} else if (typeof forgeConfig !== 'object') {
throw new Error('Expected packageJSON.config.forge to be an object or point to a requirable JS file');
Expand Down
3 changes: 2 additions & 1 deletion packages/api/core/src/util/plugin-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ export default class PluginInterface implements IForgePluginInterface {
let newStartFn;
const claimed: string[] = [];
for (const plugin of this.plugins) {
if (typeof plugin.startLogic === 'function' && plugin.startLogic !== PluginBase.prototype.startLogic) {
// FIXME(erickzhao): reference equality doesn't work due to loading Forge config from `tsx`
if (typeof plugin.startLogic === 'function' && plugin.startLogic.toString() !== PluginBase.prototype.startLogic.toString()) {
claimed.push(plugin.name);
newStartFn = plugin.startLogic;
}
Expand Down
10 changes: 10 additions & 0 deletions packages/plugin/fuses/spec/FusesPlugin.slow.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import fs from 'node:fs';
import path from 'node:path';

import { PACKAGE_MANAGERS, spawnPackageManager } from '@electron-forge/core-utils';
import { CrossSpawnOptions, spawn } from '@malept/cross-spawn-promise';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';

import { initLink } from '../../../api/core/src/api/init-scripts/init-link';
import { getElectronExecutablePath } from '../src/util/getElectronExecutablePath';

describe('FusesPlugin', () => {
Expand All @@ -25,9 +27,17 @@ describe('FusesPlugin', () => {
const outDir = path.join(appPath, 'out', 'fuses-test-app');

beforeAll(async () => {
await spawnPackageManager(PACKAGE_MANAGERS['yarn'], ['run', 'link:prepare']);
delete process.env.TS_NODE_PROJECT;
await fs.promises.copyFile(path.join(appPath, 'package.json.tmpl'), path.join(appPath, 'package.json'));
await spawn('yarn', ['install'], spawnOptions);

// Installing deps removes symlinks that were added at the start of this
// spec via `api.init`. So we should re-link local forge dependencies
// again.
process.env.LINK_FORGE_DEPENDENCIES_ON_INIT = '1';
await initLink(PACKAGE_MANAGERS['yarn'], appPath);
delete process.env.LINK_FORGE_DEPENDENCIES_ON_INIT;
});

afterAll(async () => {
Expand Down
Loading