Skip to content

Commit

Permalink
chore: Stop using node-pty in Electron renderer process
Browse files Browse the repository at this point in the history
  • Loading branch information
starpit committed Jun 9, 2021
1 parent 48fe911 commit 8bb4647
Show file tree
Hide file tree
Showing 34 changed files with 854 additions and 782 deletions.
1,062 changes: 476 additions & 586 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@
"@types/needle": "2.5.1",
"@types/node": "12.12.31",
"@types/pluralize": "0.0.29",
"@types/react": "17.0.8",
"@types/react-dom": "17.0.5",
"@types/react": "17.0.9",
"@types/react-dom": "17.0.6",
"@types/resize-observer-browser": "0.1.5",
"@types/tmp": "0.2.0",
"@types/turndown": "5.0.0",
Expand All @@ -101,7 +101,7 @@
"colors": "1.4.0",
"concurrently": "6.2.0",
"debug": "4.3.1",
"electron": "10.4.7",
"electron": "11.4.7",
"eslint": "6.8.0",
"eslint-plugin-header": "3.1.1",
"eslint-config-prettier": "6.10.1",
Expand All @@ -119,7 +119,7 @@
"nyc": "14.1.1",
"prettier": "1.19.1",
"properties-parser": "0.3.1",
"spectron": "12.0.0",
"spectron": "13.0.0",
"tmp": "0.2.1",
"typescript": "4.3.2",
"uuid": "8.3.2"
Expand Down
22 changes: 7 additions & 15 deletions packages/builder/bin/pty-rebuild.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,17 @@ if [ "$1" = "electron" ]; then
ARCH=$(node -e 'console.log(require("os").arch())')
TARGET="${PLATFORM}-${ARCH}"
echo "node-pty PLATFORM=$TARGET"
mkdir -p node_modules/node-pty-prebuilt-multiarch/build/Release
rm -f node_modules/node-pty-prebuilt-multiarch/build/Release/*
cp node_modules/@kui-shell/builder/dist/electron/vendor/node-pty-prebuilt-multiarch/build/$TARGET/electron/* node_modules/node-pty-prebuilt-multiarch/build/Release
gunzip node_modules/node-pty-prebuilt-multiarch/build/Release/*.gz
ls node_modules/node-pty-prebuilt-multiarch/build/Release
mkdir -p node_modules/node-pty/build/Release
rm -f node_modules/node-pty/build/Release/*
cp node_modules/@kui-shell/builder/dist/electron/vendor/node-pty/build/$TARGET/electron/* node_modules/node-pty/build/Release
gunzip node_modules/node-pty/build/Release/*.gz
ls node_modules/node-pty/build/Release
else
if [ -e ./node_modules/.bin/rc ] && [ ! -L ./node_modules/.bin/rc ]; then
echo "rc is not a symlink"
(cd node_modules/.bin && rm -f rc && node -e 'require("fs").symlinkSync("../rc/cli.js", "rc")')
fi
if [ -e ./node_modules/.bin/prebuild-install ] && [ ! -L ./node_modules/.bin/prebuild-install ]; then
echo "prebuild-install is not a symlink"
(cd node_modules/.bin && rm -f prebuild-install && node -e 'require("fs").symlinkSync("../prebuild-install/bin.js", "prebuild-install")')
fi

cd node_modules/node-pty-prebuilt-multiarch
prebuild-install --force --download --runtime $1
cd node_modules/node-pty
npm run install
fi

# Notes: why --force --download? for some reason, when run from `npm
# run ...` prebuild-install insists on the --build-from-source option,
# and nothing we do, short of --force, seems to override this
2 changes: 1 addition & 1 deletion packages/builder/dist/electron/builders/electron.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const { exec } = require('child_process')
const sign = require('./sign')
const notarize = require('./notarize')

const nodePty = 'node-pty-prebuilt-multiarch'
const nodePty = 'node-pty'

/**
* afterCopy hook to build webpack bundles.
Expand Down
18 changes: 9 additions & 9 deletions packages/builder/dist/electron/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/builder/dist/electron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"license": "Apache-2.0",
"devDependencies": {
"debug": "4.3.1",
"electron": "10.4.7",
"electron": "11.4.7",
"electron-packager": "15.2.0",
"glob": "7.1.7",
"globby": "11.0.3"
Expand Down
4 changes: 2 additions & 2 deletions packages/builder/dist/electron/vendor/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
This directory contains only prebuilt versions of
[node-pty-prebuilt-multiarch](https://github.com/oznu/node-pty-prebuilt-multiarch). The
code has not been modified from this npm release: 0.9.0-beta21.legacy
[node-pty](https://github.com/microsoft/node-pty). The
code has not been modified from this npm release: 0.10.1
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion packages/builder/npmrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
runtime=electron
target=10.0.0
target=11.0.0
build_from_source=true
disturl=https://atom.io/download/electron
6 changes: 3 additions & 3 deletions packages/builder/src/kui-dist-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ export const main = async (argv: string[]) => {
pjson.scripts.compile = 'kui-compile'
pjson.scripts.watch = 'tsc --build . --watch'
pjson.scripts['watch:webpack'] = 'kui-watch-webpack'
pjson.scripts['pty:rebuild'] = 'cd node_modules/node-pty-prebuilt-multiarch && npm run install'
pjson.scripts['pty:rebuild'] = 'cd node_modules/node-pty && npm run install'
pjson.scripts['pty:electron'] =
'if [ ! -e node_modules/node-pty-prebuilt-multiarch/.npmrc ]; then cp node_modules/@kui-shell/builder/npmrc node_modules/node-pty-prebuilt-multiarch/.npmrc && npm run pty:rebuild; fi'
'if [ ! -e node_modules/node-pty/.npmrc ]; then cp node_modules/@kui-shell/builder/npmrc node_modules/node-pty/.npmrc && npm run pty:rebuild; fi'
pjson.scripts['pty:nodejs'] =
'if [ -e node_modules/node-pty-prebuilt-multiarch/.npmrc ]; then rm -f node_modules/node-pty-prebuilt-multiarch/.npmrc; npm run pty:rebuild; fi'
'if [ -e node_modules/node-pty/.npmrc ]; then rm -f node_modules/node-pty/.npmrc; npm run pty:rebuild; fi'
pjson.scripts.start = 'npm run -s init && npm run -s pty:electron && electron . shell'
await writeFile('package.json', JSON.stringify(pjson, undefined, 2))

Expand Down
9 changes: 4 additions & 5 deletions packages/core/src/main/spawn-electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {

import windowDefaults, { popupWindowDefaults } from '../webapp/defaults'
import ISubwindowPrefs from '../models/SubwindowPrefs'
import { webpackPath } from '../plugins/path'

/**
* Keep a global reference of the window object, if you don't, the window will
Expand Down Expand Up @@ -170,6 +171,7 @@ export function createWindow(
webPreferences: {
enableRemoteModule: true,
backgroundThrottling: false,
contextIsolation: false, // prior to electron 12, nodeIntegration: true did not need this
nodeIntegration: true // prior to electron 5, this was the default
}
// titleBarStyle: process.platform === 'darwin' ? 'hiddenInset' : 'default'
Expand Down Expand Up @@ -442,10 +444,10 @@ export function createWindow(
debug('invoke', message)

try {
const mod = await import(message.module)
const mod = await import('@kui-shell/plugin-' + webpackPath(message.module) + '/dist/index.js')
debug('invoke got module')

const returnValue = await mod[message.main || 'main'](message.args)
const returnValue = await mod[message.main || 'main'](message.args, event.sender)
debug('invoke got returnValue', returnValue)

event.sender.send(
Expand Down Expand Up @@ -600,9 +602,6 @@ export async function initElectron(
debug('loading electron')
const Electron = await import('electron')
app = Electron.app
if (app) {
app.allowRendererProcessReuse = false
}
}

// deal with multiple processes
Expand Down
77 changes: 42 additions & 35 deletions packages/test/src/api/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,46 @@ const prepareElectron = (popup: string[]) => {
return new Application(opts)
}

/** Add app.client commands */
function addCommands(ctx: ISuite) {
// add an isActive command; isFocused is not what we want
if (ctx.app && ctx.app.client) {
// ref: https://github.com/webdriverio/webdriverio/issues/1362#issuecomment-224042781
ctx.app.client.addCommand('isActive', selector => {
return ctx.app.client.execute(selector => {
const focused = document.activeElement

if (!focused || focused === document.body) {
return false
} else if (document.querySelector) {
return document.querySelector(selector) === focused
}

return false
}, selector)
})
}
}

/** restart the app */
export const restart = async (ctx: ISuite) => {
try {
await ctx.app.restart()
addCommands(ctx)
} catch (err) {
const errorIsNavigatedError: boolean =
err.message.includes('Inspected target navigated or closed') ||
err.message.includes('cannot determine loading status') ||
err.message.includes('Inspected target navigated or closed')

if (!errorIsNavigatedError) {
throw err
}
}

return CLI.waitForSession(ctx)
}

/** reload the app */
export const refresh = async (ctx: ISuite, wait = true, clean = false) => {
try {
Expand Down Expand Up @@ -319,23 +359,8 @@ export const before = (ctx: ISuite, options?: BeforeOptions): HookFunction => {
ctx.timeout(process.env.TIMEOUT || 60000)
await start()

// add an isActive command; isFocused is not what we want
if (ctx.app && ctx.app.client) {
// ref: https://github.com/webdriverio/webdriverio/issues/1362#issuecomment-224042781
ctx.app.client.addCommand('isActive', selector => {
return ctx.app.client.execute(selector => {
const focused = document.activeElement

if (!focused || focused === document.body) {
return false
} else if (document.querySelector) {
return document.querySelector(selector) === focused
}

return false
}, selector)
})
}
// add app.client commands
addCommands(ctx)

// see https://github.com/electron-userland/spectron/issues/763
// and https://github.com/webdriverio/webdriverio/issues/6092
Expand Down Expand Up @@ -505,24 +530,6 @@ export const oops = (ctx: ISuite, wait = false) => async (err: Error) => {
// throw err
}

/** restart the app */
export const restart = async (ctx: ISuite) => {
try {
await ctx.app.restart()
} catch (err) {
const errorIsNavigatedError: boolean =
err.message.includes('Inspected target navigated or closed') ||
err.message.includes('cannot determine loading status') ||
err.message.includes('Inspected target navigated or closed')

if (!errorIsNavigatedError) {
throw err
}
}

return CLI.waitForSession(ctx)
}

/** only execute the test in local */
export const localIt = (msg: string, func: Func) => {
if (process.env.MOCHA_RUN_TARGET !== 'webpack') return it(msg, func)
Expand Down
6 changes: 3 additions & 3 deletions packages/webpack/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ plugins.push({
console.log('webpack plugins', plugins)

// Notes: when !inBrowser, we want electron to pull
// node-pty-prebuilt-multiarch in as a commonjs external module; this
// node-pty in as a commonjs external module; this
// is because node-pty has binary bits, and we are building one set of
// bundles for all electron platforms. If, in the future, we decide to
// rebuild the bundles for each platform, we can remove this 'commonjs
Expand All @@ -319,7 +319,7 @@ console.log('webpack plugins', plugins)
// the 'commonjs node-pty' syntax, see
// https://github.com/webpack/webpack/issues/4238
const externals = !inBrowser
? { 'node-pty-prebuilt-multiarch': 'commonjs node-pty-prebuilt-multiarch' }
? { 'node-pty': 'commonjs node-pty' }
: [
'tape', // modules/composer/node_modules/safer-buffer
'dns', // modules/openwhisk/node_modules/retry/example/dns.js
Expand All @@ -328,7 +328,7 @@ const externals = !inBrowser
'request', // needed by some apache-composer samples
'babel-core/register', // wskflow
'aws-sdk', // wskflow
'node-pty-prebuilt-multiarch', // bash-like
'node-pty', // bash-like
'./es6/crc9_1wire', // k8s
'./es6/crc17_xmodem', // k8s, openwhisk
'./es6/crc17_modbus', // k8s, openwhisk
Expand Down
6 changes: 3 additions & 3 deletions plugins/plugin-bash-like/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"cookie": "0.4.1",
"debug": "4.3.1",
"globby": "11.0.3",
"node-pty-prebuilt-multiarch": "0.9.0",
"node-pty": "0.10.1",
"shelljs": "0.8.4",
"slash": "3.0.0",
"speed-date": "1.0.0",
Expand All @@ -46,7 +46,7 @@
"marked",
"xterm",
"ws",
"node-pty-prebuilt-multiarch"
"node-pty"
],
"proxy": [
"asciidoctor.js",
Expand All @@ -55,7 +55,7 @@
],
"webpack": [
"ws",
"node-pty-prebuilt-multiarch"
"node-pty"
]
}
},
Expand Down
2 changes: 2 additions & 0 deletions plugins/plugin-bash-like/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ export { StdioChannelWebsocketSide } from './pty/stdio-channel'
export { getSessionForTab } from './pty/session'
export { dispatchToShell as doExecWithPty, doExecWithStdoutViaPty } from './lib/cmds/catchall'
export { getTabState } from './tab-state'

export { initMainPty } from './pty/electron-main-channel'
Loading

0 comments on commit 8bb4647

Please sign in to comment.