From 7668331c9d9b758d6d19f4bbd2a09608857c723d Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Tue, 7 Nov 2023 10:16:42 +0100 Subject: [PATCH 1/2] test(metro-plugin-duplicates-checker): address pnpm issues --- .changeset/rich-adults-mix.md | 5 + packages/cli/test/__mocks__/fs.js | 21 +---- .../package.json | 2 +- .../src/gatherModules.ts | 2 +- .../test/__mocks__/fs.js | 31 +++--- .../__snapshots__/gatherModules.test.ts.snap | 35 ------- .../test/checkForDuplicatePackages.test.ts | 20 ++++ .../test/gatherModules.test.ts | 50 +++++++++- .../test/testData.ts | 94 +++++++++++++++---- yarn.lock | 2 +- 10 files changed, 168 insertions(+), 94 deletions(-) create mode 100644 .changeset/rich-adults-mix.md delete mode 100644 packages/metro-plugin-duplicates-checker/test/__snapshots__/gatherModules.test.ts.snap diff --git a/.changeset/rich-adults-mix.md b/.changeset/rich-adults-mix.md new file mode 100644 index 000000000..5e367845e --- /dev/null +++ b/.changeset/rich-adults-mix.md @@ -0,0 +1,5 @@ +--- +"@rnx-kit/metro-plugin-duplicates-checker": patch +--- + +Fix error message not containing the path we're trying to resolve diff --git a/packages/cli/test/__mocks__/fs.js b/packages/cli/test/__mocks__/fs.js index fd6ab6d00..c219a6c24 100644 --- a/packages/cli/test/__mocks__/fs.js +++ b/packages/cli/test/__mocks__/fs.js @@ -1,20 +1 @@ -const fs = jest.createMockFromModule("fs"); - -const { vol } = require("memfs"); - -/** @type {(newMockFiles: { [filename: string]: string }) => void} */ -fs.__setMockFiles = (files) => { - vol.reset(); - vol.fromJSON(files); -}; - -fs.__toJSON = () => vol.toJSON(); - -fs.lstat = (...args) => Promise.resolve(vol.lstat(...args)); -fs.lstatSync = (...args) => vol.lstatSync(...args); -fs.mkdirSync = (...args) => vol.mkdirSync(...args); -fs.readFileSync = (...args) => vol.readFileSync(...args); -fs.stat = (...args) => Promise.resolve(vol.stat(...args)); -fs.statSync = (...args) => vol.statSync(...args); - -module.exports = fs; +module.exports = require("@rnx-kit/metro-plugin-duplicates-checker/test/__mocks__/fs"); diff --git a/packages/metro-plugin-duplicates-checker/package.json b/packages/metro-plugin-duplicates-checker/package.json index e81c4f0c9..7ad86496f 100644 --- a/packages/metro-plugin-duplicates-checker/package.json +++ b/packages/metro-plugin-duplicates-checker/package.json @@ -36,9 +36,9 @@ "@types/node": "^18.0.0", "eslint": "^8.0.0", "jest": "^29.2.1", + "memfs": "^4.0.0", "metro": "^0.76.5", "metro-source-map": "^0.76.8", - "pkg-dir": "^5.0.0", "prettier": "^3.0.0", "typescript": "^5.0.0" }, diff --git a/packages/metro-plugin-duplicates-checker/src/gatherModules.ts b/packages/metro-plugin-duplicates-checker/src/gatherModules.ts index 2f4768ab1..db6aa1ded 100644 --- a/packages/metro-plugin-duplicates-checker/src/gatherModules.ts +++ b/packages/metro-plugin-duplicates-checker/src/gatherModules.ts @@ -23,7 +23,7 @@ export function normalizePath(p: string): string { export function resolveModule(source: string): ModuleInfo { const pkg = findPackageDir(source); if (!pkg) { - throw new Error(`Unable to find package '${pkg}'`); + throw new Error(`Unable to resolve module '${source}'`); } const { name, version } = readPackage(pkg); diff --git a/packages/metro-plugin-duplicates-checker/test/__mocks__/fs.js b/packages/metro-plugin-duplicates-checker/test/__mocks__/fs.js index 14df74ccd..597001234 100644 --- a/packages/metro-plugin-duplicates-checker/test/__mocks__/fs.js +++ b/packages/metro-plugin-duplicates-checker/test/__mocks__/fs.js @@ -1,23 +1,22 @@ const fs = jest.createMockFromModule("fs"); -const actualFs = jest.requireActual("fs"); -// Under normal circumstances, this extra copy of '@react-native/polyfills' -// should not be installed. -const extraPolyfills = - "/packages/test-app/node_modules/@react-native/polyfills"; +const { vol } = require("memfs"); -fs.readFileSync = actualFs.readFileSync; - -fs.realpathSync = actualFs.realpathSync; -const actualRealpathSyncNative = actualFs.realpathSync.native; -fs.realpathSync.native = (path, ...args) => { - if (path.replace(/\\/g, "/").endsWith(extraPolyfills)) { - return path; - } else { - return actualRealpathSyncNative(path, ...args); - } +/** @type {(newMockFiles: { [filename: string]: string }) => void} */ +fs.__setMockFiles = (files) => { + vol.reset(); + vol.fromJSON(files); }; -fs.statSync = actualFs.statSync; // Used by 'pkg-dir' +fs.__toJSON = () => vol.toJSON(); + +fs.lstat = (...args) => vol.lstat(...args); +fs.lstatSync = (...args) => vol.lstatSync(...args); +fs.mkdirSync = (...args) => vol.mkdirSync(...args); +fs.readFileSync = (...args) => vol.readFileSync(...args); +fs.realpathSync = (...args) => vol.realpathSync(...args); +fs.realpathSync.native = (...args) => vol.realpathSync(...args); +fs.stat = (...args) => vol.stat(...args); +fs.statSync = (...args) => vol.statSync(...args); module.exports = fs; diff --git a/packages/metro-plugin-duplicates-checker/test/__snapshots__/gatherModules.test.ts.snap b/packages/metro-plugin-duplicates-checker/test/__snapshots__/gatherModules.test.ts.snap deleted file mode 100644 index 2edf4fe64..000000000 --- a/packages/metro-plugin-duplicates-checker/test/__snapshots__/gatherModules.test.ts.snap +++ /dev/null @@ -1,35 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`gatherModulesFromGraph() builds module map from a basic source map 1`] = ` -[ - "react-native", - "rnx-kit", -] -`; - -exports[`gatherModulesFromSourceMap() builds module map from a basic source map 1`] = ` -[ - "@babel/runtime", - "abort-controller", - "anser", - "base64-js", - "event-target-shim", - "invariant", - "metro", - "nullthrows", - "object-assign", - "pretty-format", - "promise", - "prop-types", - "react", - "react-devtools-core", - "react-is", - "react-native", - "react-refresh", - "regenerator-runtime", - "rnx-kit", - "scheduler", - "stacktrace-parser", - "whatwg-fetch", -] -`; diff --git a/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts b/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts index 1c37de5a8..eac2ffe91 100644 --- a/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts +++ b/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts @@ -8,9 +8,13 @@ import { } from "../src/checkForDuplicatePackages"; import { bundleGraph, + bundleGraphFS, bundleGraphWithDuplicates, + bundleGraphWithDuplicatesFS, bundleSourceMap, + bundleSourceMapFS, bundleSourceMapWithDuplicates, + bundleSourceMapWithDuplicatesFS, repoRoot, } from "./testData"; @@ -163,6 +167,8 @@ describe("detectDuplicatePackages()", () => { }); describe("checkForDuplicateDependencies()", () => { + const fs = require("fs"); + const consoleErrorSpy = jest.spyOn(global.console, "error"); const consoleWarnSpy = jest.spyOn(global.console, "warn"); @@ -172,10 +178,13 @@ describe("checkForDuplicateDependencies()", () => { }); afterAll(() => { + fs.__setMockFiles(); jest.clearAllMocks(); }); test("checkForDuplicateDependencies", () => { + fs.__setMockFiles(bundleGraphFS); + expect(checkForDuplicateDependencies(bundleGraph)).toEqual({ banned: 0, duplicates: 0, @@ -183,6 +192,8 @@ describe("checkForDuplicateDependencies()", () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); expect(consoleWarnSpy).not.toHaveBeenCalled(); + fs.__setMockFiles(bundleGraphWithDuplicatesFS); + expect(checkForDuplicateDependencies(bundleGraphWithDuplicates)).toEqual({ banned: 0, duplicates: 1, @@ -193,11 +204,20 @@ describe("checkForDuplicateDependencies()", () => { }); describe("checkForDuplicatePackages()", () => { + const fs = require("fs"); + + afterAll(() => fs.__setMockFiles()); + test("checkForDuplicatePackages", () => { + fs.__setMockFiles(bundleSourceMapFS); + expect(checkForDuplicatePackages(bundleSourceMap)).toEqual({ banned: 0, duplicates: 0, }); + + fs.__setMockFiles(bundleSourceMapWithDuplicatesFS); + expect(checkForDuplicatePackages(bundleSourceMapWithDuplicates)).toEqual({ banned: 0, duplicates: 1, diff --git a/packages/metro-plugin-duplicates-checker/test/gatherModules.test.ts b/packages/metro-plugin-duplicates-checker/test/gatherModules.test.ts index 33d1d684e..19e1f2188 100644 --- a/packages/metro-plugin-duplicates-checker/test/gatherModules.test.ts +++ b/packages/metro-plugin-duplicates-checker/test/gatherModules.test.ts @@ -5,7 +5,14 @@ import { normalizePath, resolveModule, } from "../src/gatherModules"; -import { bundleGraph, bundleSourceMap } from "./testData"; +import { + bundleGraph, + bundleGraphFS, + bundleSourceMap, + bundleSourceMapFS, +} from "./testData"; + +jest.mock("fs"); describe("normalizePath()", () => { test("trims Webpack URLs", () => { @@ -23,23 +30,58 @@ describe("normalizePath()", () => { describe("resolveModule()", () => { test("throws if a package is not found", () => { expect(() => resolveModule("/this-package-does-not-exist")).toThrow( - "Unable to find package" + "Unable to resolve module" ); }); }); describe("gatherModulesFromGraph()", () => { + const fs = require("fs"); + + afterAll(() => fs.__setMockFiles()); + test("builds module map from a basic source map", () => { + fs.__setMockFiles(bundleGraphFS); + const modules = gatherModulesFromGraph(bundleGraph, {}); - expect(Object.keys(modules).sort()).toMatchSnapshot(); + + expect(Object.keys(modules).sort()).toEqual(["react-native"]); }); }); describe("gatherModulesFromSourceMap()", () => { + const fs = require("fs"); + + afterAll(() => fs.__setMockFiles()); + test("builds module map from a basic source map", () => { + fs.__setMockFiles(bundleSourceMapFS); + const modules = gatherModulesFromSources(bundleSourceMap.sources, {}); - expect(Object.keys(modules).sort()).toMatchSnapshot(); + expect(Object.keys(modules).sort()).toEqual([ + "@babel/runtime", + "abort-controller", + "anser", + "base64-js", + "event-target-shim", + "invariant", + "metro", + "nullthrows", + "object-assign", + "pretty-format", + "promise", + "prop-types", + "react", + "react-devtools-core", + "react-is", + "react-native", + "react-refresh", + "regenerator-runtime", + "scheduler", + "stacktrace-parser", + "whatwg-fetch", + ]); Object.keys(modules).forEach((name) => { expect(Object.keys(modules[name]).length).toBe(1); diff --git a/packages/metro-plugin-duplicates-checker/test/testData.ts b/packages/metro-plugin-duplicates-checker/test/testData.ts index bbb67948f..443682f86 100644 --- a/packages/metro-plugin-duplicates-checker/test/testData.ts +++ b/packages/metro-plugin-duplicates-checker/test/testData.ts @@ -2,37 +2,56 @@ import type { Graph, Module } from "@rnx-kit/metro-serializer"; import type { BasicSourceMap } from "metro-source-map"; -import path from "path"; +import * as path from "node:path"; const mockModule = {} as Module; -const pkgDir = jest.requireActual("pkg-dir"); -export const repoRoot = pkgDir.sync(path.resolve(__dirname, "..", "..")); +export const repoRoot = path.resolve(__dirname, "..", "..", ".."); + +function makeManifest(name, version = "0.0.0") { + return JSON.stringify({ name, version }); +} export const bundleGraph: Graph = { dependencies: new Map([ [`${repoRoot}/packages/test-app/lib/src/index.js`, mockModule], [`${repoRoot}/node_modules/react-native/index.js`, mockModule], - [`${repoRoot}/node_modules/fbjs/lib/warning.js`, mockModule], ]), importBundleNames: new Set(), entryPoints: [], }; +export const bundleGraphFS: Record = { + [`${repoRoot}/packages/test-app/package.json`]: + makeManifest("@rnx-kit/test-app"), + [`${repoRoot}/node_modules/react-native/package.json`]: + makeManifest("react-native"), +}; + export const bundleGraphWithDuplicates: Graph = { dependencies: new Map([ - [`${repoRoot}/packages/test-app/lib/src/index.js`, mockModule], [ `${repoRoot}/node_modules/@rnx-kit/build/node_modules/chalk/source/index.js`, mockModule, ], [`${repoRoot}/node_modules/chalk/source/index.js`, mockModule], [`${repoRoot}/node_modules/react-native/index.js`, mockModule], + [`${repoRoot}/packages/test-app/lib/src/index.js`, mockModule], ]), importBundleNames: new Set(), entryPoints: [], }; +export const bundleGraphWithDuplicatesFS: Record = { + [`${repoRoot}/node_modules//@rnx-kit/build/node_modules/chalk/package.json`]: + makeManifest("chalk"), + [`${repoRoot}/node_modules/chalk/package.json`]: makeManifest("chalk"), + [`${repoRoot}/node_modules/react-native/package.json`]: + makeManifest("react-native"), + [`${repoRoot}/packages/test-app/package.json`]: + makeManifest("@rnx-kit/test-app"), +}; + export const bundleSourceMap: BasicSourceMap = { version: 3, sources: [ @@ -45,8 +64,6 @@ export const bundleSourceMap: BasicSourceMap = { `${repoRoot}/node_modules/react-native/index.js`, `${repoRoot}/node_modules/invariant/browser.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/warnOnce.js`, - `${repoRoot}/node_modules/fbjs/lib/warning.js`, - `${repoRoot}/node_modules/fbjs/lib/emptyFunction.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/AccessibilityInfo/AccessibilityInfo.ios.js`, `${repoRoot}/node_modules/@babel/runtime/helpers/interopRequireDefault.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/AccessibilityInfo/NativeAccessibilityManager.js`, @@ -176,9 +193,6 @@ export const bundleSourceMap: BasicSourceMap = { `${repoRoot}/node_modules/react-native/Libraries/Core/setUpTimers.js`, `${repoRoot}/node_modules/react-native/Libraries/Core/Timers/JSTimers.js`, `${repoRoot}/node_modules/react-native/Libraries/Core/Timers/NativeTiming.js`, - `${repoRoot}/node_modules/fbjs/lib/performanceNow.js`, - `${repoRoot}/node_modules/fbjs/lib/performance.js`, - `${repoRoot}/node_modules/fbjs/lib/ExecutionEnvironment.js`, `${repoRoot}/node_modules/react-native/Libraries/Core/setUpXHR.js`, `${repoRoot}/node_modules/react-native/Libraries/Network/XMLHttpRequest.js`, `${repoRoot}/node_modules/react-native/Libraries/Blob/BlobManager.js`, @@ -224,7 +238,6 @@ export const bundleSourceMap: BasicSourceMap = { `${repoRoot}/node_modules/react-native/Libraries/Utilities/DevSettings.js`, `${repoRoot}/node_modules/react-native/Libraries/NativeModules/specs/NativeDevSettings.js`, `${repoRoot}/node_modules/metro/src/lib/bundle-modules/HMRClient.js`, - `${repoRoot}/node_modules/eventemitter3/index.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/LoadingView.ios.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/NativeDevLoadingView.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/HMRClientProdShim.js`, @@ -298,8 +311,6 @@ export const bundleSourceMap: BasicSourceMap = { `${repoRoot}/node_modules/react-native/Libraries/Components/AppleTV/NativeTVNavigationEventEmitter.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/Sound/SoundManager.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/Sound/NativeSoundManager.js`, - `${repoRoot}/node_modules/fbjs/lib/keyMirror.js`, - `${repoRoot}/node_modules/fbjs/lib/invariant.js`, `${repoRoot}/node_modules/nullthrows/nullthrows.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/Touchable/TouchableNativeFeedback.js`, `${repoRoot}/node_modules/react-native/Libraries/Pressability/Pressability.js`, @@ -504,9 +515,6 @@ export const bundleSourceMap: BasicSourceMap = { `${repoRoot}/node_modules/react-native/Libraries/Components/StatusBar/StatusBarIOS.js`, `${repoRoot}/node_modules/react-native/Libraries/Components/ToastAndroid/ToastAndroid.ios.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/useColorScheme.js`, - `${repoRoot}/node_modules/use-subscription/index.js`, - `${repoRoot}/node_modules/use-subscription/cjs/use-subscription.production.min.js`, - `${repoRoot}/node_modules/use-subscription/cjs/use-subscription.development.js`, `${repoRoot}/node_modules/react-native/Libraries/Utilities/useWindowDimensions.js`, `${repoRoot}/node_modules/react-native/Libraries/Vibration/Vibration.js`, `${repoRoot}/node_modules/react-native/Libraries/Vibration/NativeVibration.js`, @@ -529,6 +537,50 @@ export const bundleSourceMap: BasicSourceMap = { names: [], }; +export const bundleSourceMapFS: Record = { + [`${repoRoot}/node_modules/@babel/runtime/package.json`]: + makeManifest("@babel/runtime"), + [`${repoRoot}/node_modules/abort-controller/package.json`]: + makeManifest("abort-controller"), + [`${repoRoot}/node_modules/anser/package.json`]: makeManifest("anser"), + [`${repoRoot}/node_modules/base64-js/package.json`]: + makeManifest("base64-js"), + [`${repoRoot}/node_modules/event-target-shim/package.json`]: + makeManifest("event-target-shim"), + [`${repoRoot}/node_modules/invariant/package.json`]: + makeManifest("invariant"), + [`${repoRoot}/node_modules/metro/package.json`]: makeManifest("metro"), + [`${repoRoot}/node_modules/nullthrows/package.json`]: + makeManifest("nullthrows"), + [`${repoRoot}/node_modules/object-assign/package.json`]: + makeManifest("object-assign"), + [`${repoRoot}/node_modules/promise/package.json`]: makeManifest("promise"), + [`${repoRoot}/node_modules/prop-types/package.json`]: + makeManifest("prop-types"), + [`${repoRoot}/node_modules/react-devtools-core/package.json`]: makeManifest( + "react-devtools-core" + ), + [`${repoRoot}/node_modules/react-is/package.json`]: makeManifest("react-is"), + [`${repoRoot}/node_modules/react-native/node_modules/pretty-format/package.json`]: + makeManifest("pretty-format"), + [`${repoRoot}/node_modules/react-native/package.json`]: + makeManifest("react-native"), + [`${repoRoot}/node_modules/react-refresh/package.json`]: + makeManifest("react-refresh"), + [`${repoRoot}/node_modules/react/package.json`]: makeManifest("react"), + [`${repoRoot}/node_modules/regenerator-runtime/package.json`]: makeManifest( + "regenerator-runtime" + ), + [`${repoRoot}/node_modules/scheduler/package.json`]: + makeManifest("scheduler"), + [`${repoRoot}/node_modules/stacktrace-parser/package.json`]: + makeManifest("stacktrace-parser"), + [`${repoRoot}/node_modules/whatwg-fetch/package.json`]: + makeManifest("whatwg-fetch"), + [`${repoRoot}/packages/test-app/package.json`]: + makeManifest("@rnx-kit/test-app"), +}; + export const bundleSourceMapWithDuplicates: BasicSourceMap = { version: 3, sources: [ @@ -540,3 +592,13 @@ export const bundleSourceMapWithDuplicates: BasicSourceMap = { mappings: "", names: [], }; + +export const bundleSourceMapWithDuplicatesFS: Record = { + [`${repoRoot}/node_modules/@rnx-kit/build/node_modules/chalk/package.json`]: + makeManifest("chalk"), + [`${repoRoot}/node_modules/chalk/package.json`]: makeManifest("chalk"), + [`${repoRoot}/node_modules/react-native/package.json`]: + makeManifest("react-native"), + [`${repoRoot}/packages/test-app/package.json`]: + makeManifest("@rnx-kit/test-app"), +}; diff --git a/yarn.lock b/yarn.lock index a7f5b997c..9630c5f1a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3801,9 +3801,9 @@ __metadata: "@types/node": ^18.0.0 eslint: ^8.0.0 jest: ^29.2.1 + memfs: ^4.0.0 metro: ^0.76.5 metro-source-map: ^0.76.8 - pkg-dir: ^5.0.0 prettier: ^3.0.0 typescript: ^5.0.0 bin: From bd1b59a79ccd8fd66102879f3e97b54a5df45539 Mon Sep 17 00:00:00 2001 From: Tommy Nguyen <4123478+tido64@users.noreply.github.com> Date: Tue, 7 Nov 2023 11:05:03 +0100 Subject: [PATCH 2/2] fixup! test(metro-plugin-duplicates-checker): address pnpm issues --- .../test/checkForDuplicatePackages.test.ts | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts b/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts index eac2ffe91..56379cc0a 100644 --- a/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts +++ b/packages/metro-plugin-duplicates-checker/test/checkForDuplicatePackages.test.ts @@ -15,46 +15,9 @@ import { bundleSourceMapFS, bundleSourceMapWithDuplicates, bundleSourceMapWithDuplicatesFS, - repoRoot, } from "./testData"; jest.mock("fs"); -jest.mock("@rnx-kit/tools-node/package"); - -// Under normal circumstances, this extra copy of '@react-native/polyfills' -// should not be installed. -const extraPolyfills = `${repoRoot.replace( - /\\/g, - "/" -)}/packages/test-app/node_modules/@react-native/polyfills`; -require("@rnx-kit/tools-node/package").findPackageDir = jest - .fn() - .mockImplementation((startDir) => { - switch (startDir) { - // Under normal circumstances, this extra copy of '@react-native/polyfills' - // should not be installed. - case `${extraPolyfills}/index.js`: - return extraPolyfills; - default: - return jest - .requireActual("@rnx-kit/tools-node/package") - .findPackageDir(startDir); - } - }); -require("@rnx-kit/tools-node/package").readPackage = jest - .fn() - .mockImplementation((path) => { - if (path.replace(/\\/g, "/").includes(extraPolyfills)) { - return { - name: "@react-native/polyfills", - version: "1.0.0", - }; - } else { - return jest - .requireActual("@rnx-kit/tools-node/package") - .readPackage(path); - } - }); const defaultOptions: Options = { ignoredModules: [],