From 26d633cfcd94dbea2f226d6648b687399b1f8da6 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Fri, 21 Jun 2024 12:32:49 +0900 Subject: [PATCH] fix: nyc coverage compatibility (#54) --- package.json | 1 + pnpm-lock.yaml | 25 ++++++++++ src/cjs/api/module-extensions.ts | 84 ++++++++++++++++++++++++++------ tests/specs/api.ts | 41 ++++++++++++++++ 4 files changed, 136 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 29081f0b..543f5b1a 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "@types/cross-spawn": "^6.0.6", "@types/node": "^20.14.1", "@types/split2": "^4.2.3", + "append-transform": "^2.0.0", "cachedir": "^2.4.0", "chokidar": "^3.6.0", "clean-pkg-json": "^1.2.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0677a357..53b933e8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -31,6 +31,9 @@ importers: '@types/split2': specifier: ^4.2.3 version: 4.2.3 + append-transform: + specifier: ^2.0.0 + version: 2.0.0 cachedir: specifier: ^2.4.0 version: 2.4.0 @@ -1235,6 +1238,10 @@ packages: resolution: {integrity: sha512-KMReFUr0B4t+D+OBkjR3KYqvocp2XaSzO55UcB6mgQMd3KbcE+mWTyvVV7D/zsdEbNnV6acZUutkiHQXvTr1Rw==} engines: {node: '>= 8'} + append-transform@2.0.0: + resolution: {integrity: sha512-7yeyCEurROLQJFv5Xj4lEGTy0borxepjFv1g22oAdqFu//SrAlDl1O1Nxx15SH1RoliUml6p8dwJW9jvZughhg==} + engines: {node: '>=8'} + arg@5.0.2: resolution: {integrity: sha512-PYjyFOLKQ9y57JvQ6QLo8dAgNqswh8M1RMJYdQduT6xbWSgK36P/Z/v+p888pM69jMMfS8Xd8F6I1kQ/I9HUGg==} @@ -1509,6 +1516,10 @@ packages: resolution: {integrity: sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A==} engines: {node: '>=0.10.0'} + default-require-extensions@3.0.1: + resolution: {integrity: sha512-eXTJmRbm2TIt9MgWTsOH1wEuhew6XGZcMeGKCtLedIg/NCsg1iBePXkceTdK4Fii7pzmN9tGsZhKzZ4h7O/fxw==} + engines: {node: '>=8'} + defer-to-connect@2.0.1: resolution: {integrity: sha512-4tvttepXG1VaYGrRibk5EwJd1t4udunSOVMdLSAL6mId1ix438oPwPZMALY41FCijukO1L0twNcGsdzS7dHgDg==} engines: {node: '>=10'} @@ -3155,6 +3166,10 @@ packages: resolution: {integrity: sha512-vavAMRXOgBVNF6nyEEmL3DBK19iRpDcoIwW+swQ+CbGiu7lju6t+JklA1MHweoWtadgt4ISVUsXLyDq34ddcwA==} engines: {node: '>=4'} + strip-bom@4.0.0: + resolution: {integrity: sha512-3xurFv5tEgii33Zi8Jtp55wEIILR9eh34FAW00PZf+JnSsTmV/ioewSgQl97JHvgjoRGwPShsWm+IdrxB35d0w==} + engines: {node: '>=8'} + strip-final-newline@3.0.0: resolution: {integrity: sha512-dOESqjYr96iWYylGObzd39EuNTa5VJxyvVAEm5Jnh7KGo75V43Hk1odPQkNDyXNmUR6k+gEiDVXnjB8HJ3crXw==} engines: {node: '>=12'} @@ -4511,6 +4526,10 @@ snapshots: normalize-path: 3.0.0 picomatch: 2.3.1 + append-transform@2.0.0: + dependencies: + default-require-extensions: 3.0.1 + arg@5.0.2: {} argparse@2.0.1: {} @@ -4805,6 +4824,10 @@ snapshots: deepmerge@4.3.1: {} + default-require-extensions@3.0.1: + dependencies: + strip-bom: 4.0.0 + defer-to-connect@2.0.1: {} define-data-property@1.1.4: @@ -6806,6 +6829,8 @@ snapshots: strip-bom@3.0.0: {} + strip-bom@4.0.0: {} + strip-final-newline@3.0.0: {} strip-final-newline@4.0.0: {} diff --git a/src/cjs/api/module-extensions.ts b/src/cjs/api/module-extensions.ts index 30edd110..f41b74a1 100644 --- a/src/cjs/api/module-extensions.ts +++ b/src/cjs/api/module-extensions.ts @@ -23,15 +23,70 @@ const transformExtensions = [ '.mjs', ] as const; +const cloneExtensions = ( + extensions: ObjectType, +) => { + const cloneTo: ObjectType = Object.create(Object.getPrototypeOf(extensions)); + + // Preserves setters if they exist (e.g. nyc via append-transform) + const descriptors = Object.getOwnPropertyDescriptors(extensions); + for (const property in descriptors) { + if (Object.hasOwn(descriptors, property)) { + Object.defineProperty(cloneTo, property, descriptors[property]); + } + } + + return cloneTo; +}; + +const safeSet = >( + object: T, + property: keyof T, + value: T[keyof T], + descriptor?: { + enumerable?: boolean; + configurable?: boolean; + writable?: boolean; + }, +) => { + const existingDescriptor = Object.getOwnPropertyDescriptor(object, property); + + // If setter is provided, use it + if (existingDescriptor?.set) { + object[property] = value; + } else if ( + !existingDescriptor + || existingDescriptor.configurable + ) { + Object.defineProperty(object, property, { + value, + enumerable: existingDescriptor?.enumerable || descriptor?.enumerable, + writable: ( + descriptor?.writable + ?? ( + existingDescriptor + ? existingDescriptor.writable + : true + ) + ), + configurable: ( + descriptor?.configurable + ?? ( + existingDescriptor + ? existingDescriptor.configurable + : true + ) + ), + }); + } +}; + export const createExtensions = ( extendExtensions: NodeJS.RequireExtensions, namespace?: string, ) => { // Clone Module._extensions with null prototype - const extensions: NodeJS.RequireExtensions = Object.assign( - Object.create(null), - extendExtensions, - ); + const extensions = cloneExtensions(extendExtensions); const defaultLoader = extensions['.js']; @@ -105,13 +160,10 @@ export const createExtensions = ( * Any file requested with an explicit extension will be loaded using the .js loader: * https://github.com/nodejs/node/blob/e339e9c5d71b72fd09e6abd38b10678e0c592ae7/lib/internal/modules/cjs/loader.js#L430 */ - extensions['.js'] = transformer; + safeSet(extensions, '.js', transformer); for (const extension of implicitlyResolvableExtensions) { - const descriptor = Object.getOwnPropertyDescriptor(extensions, extension); - Object.defineProperty(extensions, extension, { - value: transformer, - + safeSet(extensions, extension, transformer, { /** * Registeration needs to be enumerable for some 3rd party libraries * https://github.com/gulpjs/rechoir/blob/v0.8.0/index.js#L21 (used by Webpack CLI) @@ -119,8 +171,9 @@ export const createExtensions = ( * If the extension already exists, inherit its enumerable property * If not, only expose if it's not namespaced */ - enumerable: descriptor?.enumerable || !namespace, + enumerable: !namespace, writable: true, + configurable: true, }); } @@ -133,15 +186,16 @@ export const createExtensions = ( * That said, it's actually ".js" and ".mjs" that get special treatment * rather than ".cjs" (it might as well be ".random-ext") */ - Object.defineProperty(extensions, '.mjs', { - value: transformer, - + safeSet(extensions, '.mjs', transformer, { /** - * Prevent Object.keys from detecting these extensions + * enumerable defaults to whatever is already set, but if not set, it's false + * + * This prevent Object.keys from detecting these extensions * when CJS loader iterates over the possible extensions * https://github.com/nodejs/node/blob/v22.2.0/lib/internal/modules/cjs/loader.js#L609 */ - enumerable: false, + writable: true, + configurable: true, }); return extensions; diff --git a/tests/specs/api.ts b/tests/specs/api.ts index c25f097d..9f66d9cf 100644 --- a/tests/specs/api.ts +++ b/tests/specs/api.ts @@ -1,3 +1,4 @@ +import path from 'node:path'; import { execaNode } from 'execa'; import { testSuite, expect } from 'manten'; import { createFixture } from 'fs-fixture'; @@ -94,6 +95,46 @@ export default testSuite(({ describe }, node: NodeApis) => { }); }); + test('works with append-transform (nyc)', async () => { + await using fixture = await createFixture({ + 'index.js': ` + import path from 'node:path'; + import './ts.ts' + `, + 'ts.ts': 'export const ts = "ts" as string', + 'hook.js': ` + const path = require('path'); + const appendTransform = require('append-transform') + appendTransform((code, filename) => { + if (filename.endsWith(path.sep + 'index.js')) { + console.log('js working'); + } + return code; + }); + appendTransform((code, filename) => { + if (filename.endsWith(path.sep + 'ts.ts')) { + console.log('ts working'); + } + return code; + }, '.ts'); + `, + node_modules: ({ symlink }) => symlink(path.resolve('node_modules'), 'junction'), + }); + + const { stdout } = await execaNode('./index.js', { + cwd: fixture.path, + nodePath: node.path, + nodeOptions: [ + '--require', + './hook.js', + '--require', + tsxCjsPath, + ], + }); + + expect(stdout).toBe('js working\nts working'); + }); + test('register / unregister', async () => { await using fixture = await createFixture({ 'register.cjs': `