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

feat: Node compatible ESM build #756

Merged
merged 5 commits into from
Oct 2, 2023
Merged
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
4 changes: 2 additions & 2 deletions examples/electron-react-boilerplate/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"name": "electron-react-boilerplate",
"scripts": {
"build": "concurrently \"npm run build:main\" \"npm run build:renderer\"",
"build:main": "cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.main.prod.ts",
"build:renderer": "cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.prod.ts"
"build:main": "cross-env NODE_ENV=production TS_NODE_PROJECT=tsconfig.json TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.main.prod.ts",
"build:renderer": "cross-env NODE_ENV=production TS_NODE_PROJECT=tsconfig.json TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.prod.ts"
},
"devDependencies": {
"@pmmmwh/react-refresh-webpack-plugin": "0.5.4",
Expand Down
24 changes: 12 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
"require": "./renderer/index.js",
"import": "./esm/renderer/index.js"
},
"./preload": "./preload/index.js"
"./preload": {
"require": "./preload/index.js",
"import": "./esm/preload/index.js"
}
},
"repository": "https://github.com/getsentry/sentry-electron.git",
"author": "Sentry",
Expand All @@ -35,27 +38,21 @@
},
"scripts": {
"prebuild": "yarn clean && node scripts/update-version.js",
"build": "run-p build:es6 build:esm build:preload",
"build:es6": "tsc -p tsconfig.build.json",
"build:esm": "tsc -p tsconfig.esm.json",
"build:preload": "node scripts/build-preload.js",
"build:watch": "run-p build:watch:es6 build:watch:esm",
"build:watch:es6": "tsc -p tsconfig.build.json -w --preserveWatchOutput",
"build:watch:esm": "tsc -p tsconfig.esm.json -w --preserveWatchOutput",
"build": "rollup --config rollup.config.js",
"clean": "rimraf coverage esm main preload renderer index.* integrations.* ipc.* sentry-electron*.tgz",
"prelint": "node scripts/update-version.js",
"update-electron-versions": "electron-latest-versions --start 2 > ./test/e2e/versions.json",
"update-sdk-versions": "node ./scripts/update-sdk-versions.mjs",
"lint": "run-s lint:prettier lint:eslint",
"lint:prettier": "prettier --check \"{src,test}/**/*.ts\"",
"lint:eslint": "eslint . --cache --format stylish",
"fix": "run-s fix:eslint fix:prettier",
"fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
"fix:eslint": "eslint . --format stylish --fix",
"update-electron-versions": "electron-latest-versions --start 2 > ./test/e2e/versions.json",
"update-sdk-versions": "node ./scripts/update-sdk-versions.mjs",
"pretest": "yarn build",
"test": "cross-env TS_NODE_PROJECT=tsconfig.json xvfb-maybe electron-mocha --require ts-node/register/transpile-only --timeout 120000 ./test/unit/**/*.ts",
"test": "cross-env TS_NODE_PROJECT=tsconfig.test.json xvfb-maybe electron-mocha --require ts-node/register/transpile-only --timeout 120000 ./test/unit/**/*.ts",
"pree2e": "rimraf test/e2e/dist/**/node_modules/@sentry/** test/e2e/dist/**/yarn.lock test/e2e/dist/**/package-lock.json && node scripts/clean-cache.js && yarn build && npm pack",
"e2e": "cross-env TS_NODE_PROJECT=tsconfig.json xvfb-maybe mocha --require ts-node/register/transpile-only --retries 3 ./test/e2e/*.ts"
"e2e": "cross-env TS_NODE_PROJECT=tsconfig.test.json xvfb-maybe mocha --require ts-node/register/transpile-only --retries 3 ./test/e2e/*.ts"
},
"dependencies": {
"@sentry/browser": "7.68.0",
Expand All @@ -68,6 +65,8 @@
"tslib": "^2.5.0"
},
"devDependencies": {
"@rollup/plugin-node-resolve": "^15.2.1",
"@rollup/plugin-typescript": "^11.1.4",
"@sentry-internal/eslint-config-sdk": "7.68.0",
"@sentry-internal/typescript": "7.68.0",
"@types/busboy": "^0.2.3",
Expand Down Expand Up @@ -97,6 +96,7 @@
"npm-run-all": "^4.1.5",
"prettier": "^2.8.4",
"rimraf": "^3.0.2",
"rollup": "^3.29.4",
"tmp": "^0.2.1",
"ts-node": "^10.9.1",
"typescript": "^4.9.5",
Expand Down
77 changes: 77 additions & 0 deletions rollup.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
const { builtinModules } = require('module');
const { resolve } = require('path');

const typescript = require('@rollup/plugin-typescript');

const dependencies = Object.keys(require(resolve(process.cwd(), 'package.json')).dependencies || {});
const external = [...builtinModules, 'electron', ...dependencies];

const outputOptions = {
sourcemap: true,
strict: false,
freeze: false,
externalLiveBindings: false,
generatedCode: {
preset: 'es2015',
symbols: false,
},
};

// a simple plugin that adds a package.json file with type: module
const modulePackageJson = {
name: 'package-json-module-type',
generateBundle(_, __) {
this.emitFile({
type: 'asset',
fileName: 'package.json',
source: '{"type": "module"}',
});
},
};

function transpileFiles(format, input, outDir) {
return {
input,
output: {
...outputOptions,
format,
dir: outDir,
preserveModules: true,
},
treeshake: { moduleSideEffects: false },
plugins: [
typescript({
outDir,
tsconfig: './tsconfig.build.json',
}),
format === 'esm' ? modulePackageJson : {},
],
external,
};
}

function bundlePreload(format, input, output) {
return {
input,
output: {
...outputOptions,
format,
file: output,
},
plugins: [
typescript({
tsconfig: './tsconfig.preload.json',
}),
],
external,
};
}

module.exports = [
transpileFiles('cjs', ['src/index.ts', 'src/main/index.ts', 'src/renderer/index.ts'], '.'),
transpileFiles('esm', ['src/index.ts', 'src/main/index.ts', 'src/renderer/index.ts'], './esm'),
bundlePreload('cjs', 'src/preload/index.ts', './preload/index.js'),
bundlePreload('cjs', 'src/preload/legacy.ts', './preload/legacy.js'),
bundlePreload('esm', 'src/preload/index.ts', './esm/preload/index.js'),
bundlePreload('esm', 'src/preload/legacy.ts', './esm/preload/legacy.js'),
];
45 changes: 0 additions & 45 deletions scripts/build-preload.js

This file was deleted.

16 changes: 10 additions & 6 deletions src/main/electron-normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ export const isPackaged = (() => {

/** A promise that is resolved when the app is ready */
export const whenAppReady: Promise<void> = (() => {
return app.isReady()
? Promise.resolve()
: new Promise<void>((resolve) => {
app.once('ready', () => {
resolve();
if (app) {
return app.isReady()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is app not defined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code gets loaded into the Electron renderer rather than the main process.

We have code that displays a warning if you load the wrong code into the wrong process. Rollup requires tha we target module: esnext and it then transpiles to CJS/ESM. The issue with this is that imports/side-effects are changed/moved and these bits of code get hit before our warning code!

? Promise.resolve()
: new Promise<void>((resolve) => {
app.once('ready', () => {
resolve();
});
});
});
} else {
return Promise.resolve();
}
})();

/**
Expand Down
5 changes: 2 additions & 3 deletions src/main/fs.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { app } from 'electron';
import { mkdir, readdir, readFile, rename, stat, statSync, unlink, writeFile } from 'fs';
import { mkdir, readdir, readFile, stat, statSync, unlink, writeFile } from 'fs';
import { dirname, join, resolve } from 'path';
import { promisify } from 'util';

export const sentryCachePath = join(app.getPath('userData'), 'sentry');
export const sentryCachePath = join(app ? app.getPath('userData') : '', 'sentry');

export const writeFileAsync = promisify(writeFile);
export const readFileAsync = promisify(readFile);
export const mkdirAsync = promisify(mkdir);
export const statAsync = promisify(stat);
export const unlinkAsync = promisify(unlink);
export const readDirAsync = promisify(readdir);
export const renameAsync = promisify(rename);

// mkdir with recursive was only added in Node 10+

Expand Down
33 changes: 24 additions & 9 deletions src/main/integrations/preload-injection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,34 @@ import { Integration } from '@sentry/types';
import { logger } from '@sentry/utils';
import { app } from 'electron';
import { existsSync } from 'fs';
import { isAbsolute } from 'path';
import { isAbsolute, resolve } from 'path';
import { fileURLToPath } from 'url';

import { IPCMode } from '../../common';
import { rendererRequiresCrashReporterStart } from '../electron-normalize';
import { ElectronMainOptionsInternal } from '../sdk';

// After bundling with webpack, require.resolve can return number so we include that in the types
// to ensure we check for that!
function getPreloadPath(): string | number | undefined {
try {
return rendererRequiresCrashReporterStart()
? require.resolve('../../preload/legacy.js')
: require.resolve('../../preload/index.js');
} catch (_) {
try {
// This could be ESM
const currentDir = fileURLToPath(import.meta.url);
// Use the CJS preload
return resolve(currentDir, '..', '..', '..', '..', 'preload', 'index.js');
Comment on lines +19 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this will break with any bundlers, but if the e2e tests pass we should be fine I guess.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preload injection mostly fails anyway with bundlers anyway and we fallback to custom protocol.

Once Electron v28 makes it to beta we can add some ESM main process bundler tests.

} catch (_) {
//
}
}

return undefined;
}

/**
* Injects the preload script into the provided sessions.
*
Expand Down Expand Up @@ -45,14 +67,7 @@ export class PreloadInjection implements Integration {
* Attempts to add the preload script the the provided sessions
*/
private _addPreloadToSessions(options: ElectronMainOptionsInternal): void {
let path = undefined;
try {
path = rendererRequiresCrashReporterStart()
? require.resolve('../../preload/legacy.js')
: require.resolve('../../preload/index.js');
} catch (_) {
//
}
const path = getPreloadPath();

if (path && typeof path === 'string' && isAbsolute(path) && existsSync(path)) {
for (const sesh of options.getSessions()) {
Expand Down
2 changes: 0 additions & 2 deletions src/preload/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/**
* This preload script may be used with sandbox mode enabled which means regular require is not available.
*
* npm script `build:preload` calls `node ./scripts/build-preload.js` which inlines ipc and transpiles to JavaScript
*/

import { contextBridge, ipcRenderer } from 'electron';
Expand Down
2 changes: 0 additions & 2 deletions src/preload/legacy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/**
* This preload script may be used with sandbox mode enabled which means regular require is not available.
*
* npm script `build:preload` calls `node ./scripts/build-preload.js` which inlines ipc and transpiles to JavaScript
*/

import { contextBridge, crashReporter, ipcRenderer } from 'electron';
Expand Down
20 changes: 6 additions & 14 deletions tsconfig.build.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
{
"extends": "./node_modules/@sentry-internal/typescript/tsconfig.json",
"include": [
"src/**/*.ts"
],
"exclude": [
"node_modules",
"src/preload/*.ts"
],
"include": ["src/**/*.ts"],
"exclude": ["node_modules", "src/preload/**/*.ts"],
"compilerOptions": {
"skipLibCheck": true,
"baseUrl": ".",
"lib": [
"es7",
"dom"
],
"module": "commonjs",
"lib": ["ES7", "DOM"],
"module": "ESNext",
"outDir": ".",
"rootDir": "src",
"target": "es6",
"esModuleInterop": true,
"target": "ES6",
"esModuleInterop": true
}
}
8 changes: 0 additions & 8 deletions tsconfig.esm.json

This file was deleted.

12 changes: 2 additions & 10 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
{
"extends": "./tsconfig.build.json",
"include": [
"src/**/*.ts",
"test/**/*.ts"
],
"exclude": [
"dist"
],
"compilerOptions": {
"rootDir": ".",
}
"include": ["src/**/*.ts", "test/**/*.ts"],
"exclude": ["node_modules"]
}
9 changes: 9 additions & 0 deletions tsconfig.preload.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "./tsconfig.build.json",
"include": ["src/preload/**/*.ts"],
"exclude": ["node_modules"],
"compilerOptions": {
"declaration": false,
"declarationMap": false
}
}
9 changes: 9 additions & 0 deletions tsconfig.test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "./tsconfig.build.json",
"include": ["src/**/*.ts", "test/**/*.ts"],
"exclude": ["node_modules"],
"compilerOptions": {
"module": "CommonJS",
"rootDir": "."
}
}
Loading
Loading