From 64aea8558413d72d0c910cc1c7a7afff0de52348 Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Sun, 5 Sep 2021 23:08:46 -0400 Subject: [PATCH 1/2] fix: include prefix when filtering with --only The logic currently ignores a module prefix when filtering packages passed via `--only=`. This is quite confusing: to only rebuild something like `@theia/node-pty` we must pass `--only=node-pty`. This commit adds a lookup using the prefixed name. --- src/rebuild.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rebuild.ts b/src/rebuild.ts index 56cfc37e..abae00e7 100644 --- a/src/rebuild.ts +++ b/src/rebuild.ts @@ -324,7 +324,7 @@ export class Rebuilder { } this.realModulePaths.add(realPath); - if (this.prodDeps[`${prefix}${modulePath}`] && (!this.onlyModules || this.onlyModules.includes(modulePath))) { + if (this.prodDeps[`${prefix}${modulePath}`] && (!this.onlyModules || this.onlyModules.includes(modulePath) || this.onlyModules.includes(`${prefix}${modulePath}`))) { this.rebuilds.push(() => this.rebuildModuleAt(realPath)); } From d84d91e8459e4df994d17ef7c3117f26984f4604 Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Sun, 5 Sep 2021 23:08:46 -0400 Subject: [PATCH 2/2] add tests for rebuilding scoped native modules --- test/fixture/native-app2/app/package.json | 28 +++++++++++++ .../native-app2/native-addon/binding.gyp | 8 ++++ test/fixture/native-app2/native-addon/hello.c | 23 +++++++++++ .../fixture/native-app2/native-addon/hello.js | 2 + .../native-app2/native-addon/package.json | 7 ++++ test/fixture/native-app2/package.json | 7 ++++ test/helpers/module-setup.ts | 18 ++++++--- test/rebuild.ts | 40 ++++++++++++++++++- 8 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 test/fixture/native-app2/app/package.json create mode 100644 test/fixture/native-app2/native-addon/binding.gyp create mode 100644 test/fixture/native-app2/native-addon/hello.c create mode 100644 test/fixture/native-app2/native-addon/hello.js create mode 100644 test/fixture/native-app2/native-addon/package.json create mode 100644 test/fixture/native-app2/package.json diff --git a/test/fixture/native-app2/app/package.json b/test/fixture/native-app2/app/package.json new file mode 100644 index 00000000..c5128e29 --- /dev/null +++ b/test/fixture/native-app2/app/package.json @@ -0,0 +1,28 @@ +{ + "name": "native-app", + "productName": "Native App", + "version": "1.0.0", + "description": "", + "main": "src/index.js", + "scripts": { + "start": "electron-forge start" + }, + "keywords": [], + "author": "", + "license": "MIT", + "config": { + "forge": "./forge.config.js" + }, + "devDependencies": { + "@types/node": "^12.0.10", + "@scoped/native-addon": "1.2.3", + "ffi-napi": "2.4.5" + }, + "dependencies": { + "@newrelic/native-metrics": "5.3.0", + "farmhash": "3.2.1", + "level": "6.0.0", + "native-hello-world": "2.0.0", + "ref-napi": "1" + } +} diff --git a/test/fixture/native-app2/native-addon/binding.gyp b/test/fixture/native-app2/native-addon/binding.gyp new file mode 100644 index 00000000..4dc6017f --- /dev/null +++ b/test/fixture/native-app2/native-addon/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "hello", + "sources": [ "hello.c" ] + } + ] +} diff --git a/test/fixture/native-app2/native-addon/hello.c b/test/fixture/native-app2/native-addon/hello.c new file mode 100644 index 00000000..19207c82 --- /dev/null +++ b/test/fixture/native-app2/native-addon/hello.c @@ -0,0 +1,23 @@ +#include +#include + +static napi_value Method(napi_env env, napi_callback_info info) { + napi_status status; + napi_value world; + status = napi_create_string_utf8(env, "world", 5, &world); + assert(status == napi_ok); + return world; +} + +#define DECLARE_NAPI_METHOD(name, func) \ + { name, 0, func, 0, 0, 0, napi_default, 0 } + +static napi_value Init(napi_env env, napi_value exports) { + napi_status status; + napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Method); + status = napi_define_properties(env, exports, 1, &desc); + assert(status == napi_ok); + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/fixture/native-app2/native-addon/hello.js b/test/fixture/native-app2/native-addon/hello.js new file mode 100644 index 00000000..26cb506e --- /dev/null +++ b/test/fixture/native-app2/native-addon/hello.js @@ -0,0 +1,2 @@ +const addon = require('./build/Release/hello.node'); +console.log(addon.hello()); // 'world' diff --git a/test/fixture/native-app2/native-addon/package.json b/test/fixture/native-app2/native-addon/package.json new file mode 100644 index 00000000..25c7a331 --- /dev/null +++ b/test/fixture/native-app2/native-addon/package.json @@ -0,0 +1,7 @@ +{ + "private": true, + "name": "@scoped/native-addon", + "version": "1.2.3", + "main": "index.js", + "license": "MIT" +} diff --git a/test/fixture/native-app2/package.json b/test/fixture/native-app2/package.json new file mode 100644 index 00000000..66c79637 --- /dev/null +++ b/test/fixture/native-app2/package.json @@ -0,0 +1,7 @@ +{ + "private": true, + "workspaces": [ + "native-addon", + "app" + ] +} diff --git a/test/helpers/module-setup.ts b/test/helpers/module-setup.ts index fa34f6fd..08706528 100644 --- a/test/helpers/module-setup.ts +++ b/test/helpers/module-setup.ts @@ -14,14 +14,22 @@ export function resetMSVSVersion(): void { } } -export async function resetTestModule(testModulePath: string): Promise { +export interface ResetTestModuleOptions { + packageManager?: string + fixturePath?: string +} +export async function resetTestModule(testModulePath: string, options: ResetTestModuleOptions = {}): Promise { + const { + packageManager = 'npm', + fixturePath = path.resolve(__dirname, '../../test/fixture/native-app1') + } = options; await fs.remove(testModulePath); await fs.mkdir(testModulePath, { recursive: true }); - await fs.copyFile( - path.resolve(__dirname, '../../test/fixture/native-app1/package.json'), - path.resolve(testModulePath, 'package.json') + await fs.copy( + path.resolve(fixturePath), + path.resolve(testModulePath), ); - await spawn('npm', ['install'], { cwd: testModulePath }); + await spawn(packageManager, ['install'], { cwd: testModulePath }); resetMSVSVersion(); } diff --git a/test/rebuild.ts b/test/rebuild.ts index 92fef74f..5282d5eb 100644 --- a/test/rebuild.ts +++ b/test/rebuild.ts @@ -146,7 +146,7 @@ describe('rebuilder', () => { buildPath: testModulePath, electronVersion: testElectronVersion, arch: process.arch, - onlyModules: ['ffi-napi', 'ref-napi'], // TODO: check to see if there's a bug with scoped modules + onlyModules: ['ffi-napi', 'ref-napi'], force: true }); let built = 0; @@ -194,4 +194,42 @@ describe('rebuilder', () => { await expectNativeModuleToBeRebuilt(testModulePath, 'ffi-napi'); }); }); + + describe('only rebuild (with scoped module)', function() { + this.timeout(2 * MINUTES_IN_MILLISECONDS); + + before(async () => await resetTestModule(testModulePath, { + packageManager: 'yarn', + fixturePath: path.resolve(__dirname, 'fixture/native-app2'), + })); + after(async () => await cleanupTestModule(testModulePath)); + + it('should rebuild multiple specified modules via --only option even when not prefixed with its scope', async () => { + const rebuilder = rebuild({ + buildPath: path.resolve(testModulePath, 'app'), + electronVersion: testElectronVersion, + arch: process.arch, + onlyModules: ['ffi-napi', 'ref-napi', 'native-addon'], + force: true + }); + let built = 0; + rebuilder.lifecycle.on('module-done', () => built++); + await rebuilder; + expect(built).to.equal(3); + }); + + it('should rebuild multiple specified modules via --only option when prefixed with its scope', async () => { + const rebuilder = rebuild({ + buildPath: path.resolve(testModulePath, 'app'), + electronVersion: testElectronVersion, + arch: process.arch, + onlyModules: ['ffi-napi', 'ref-napi', '@scoped/native-addon'], + force: true + }); + let built = 0; + rebuilder.lifecycle.on('module-done', () => built++); + await rebuilder; + expect(built).to.equal(3); + }); + }); });