From b691e7fe4fe699f72d0ac30ccfb28c2f3c7d01c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Thu, 7 Nov 2024 14:49:54 +0000 Subject: [PATCH 1/8] Add a test case that should fail but doesn't! I fear that Jest may be transpiling the ESM loader. I'll need to try using another test runner (Vitest or Node) --- packages/load/tests/custom-loader.mjs | 20 +++++++++++++++++++ packages/load/tests/use-custom-loader.spec.ts | 12 +++++------ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 packages/load/tests/custom-loader.mjs diff --git a/packages/load/tests/custom-loader.mjs b/packages/load/tests/custom-loader.mjs new file mode 100644 index 00000000000..eb0a169c24b --- /dev/null +++ b/packages/load/tests/custom-loader.mjs @@ -0,0 +1,20 @@ +import { buildSchema, parse } from 'graphql'; + +const loader = function (_, { customLoaderContext: { loaderType }, fooFieldName }) { + if (loaderType === 'documents') { + return parse(/* GraphQL */ ` + query TestQuery { + ${fooFieldName} + } + `); + } else if (loaderType === 'schema') { + return buildSchema(/* GraphQL */ ` + type Query { + ${fooFieldName}: String + } + `); + } + return 'I like turtles'; +}; + +export default loader; diff --git a/packages/load/tests/use-custom-loader.spec.ts b/packages/load/tests/use-custom-loader.spec.ts index 7fd902f0647..a06d0f92fd0 100644 --- a/packages/load/tests/use-custom-loader.spec.ts +++ b/packages/load/tests/use-custom-loader.spec.ts @@ -1,9 +1,9 @@ -import { getCustomLoaderByPath } from '../src/utils/custom-loader.js'; +import { useCustomLoader } from '../src/utils/custom-loader.js'; -describe('getCustomLoaderByPath', () => { - it('can load a custom loader from a file path', async () => { - const loader = getCustomLoaderByPath('./custom-loader.js', __dirname); - expect(loader).toBeDefined(); - expect(loader('some-name', { customLoaderContext: {} })).toEqual('I like turtles'); +describe('useCustomLoader', () => { + it.each(['js', 'mjs'])('can load a custom loader from a file path', async (extension: string) => { + const loader = await useCustomLoader(`./custom-loader.${extension}`, __dirname); + const result = await loader('some-name', { customLoaderContext: {} }); + expect(result).toEqual('I like turtles'); }); }); From f02d74e81d9ea224f90b00bda64f6adba3d9e447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Thu, 7 Nov 2024 22:45:34 +0000 Subject: [PATCH 2/8] Add a version of the test to be run directly with Node This one fails like it should for ESM. --- package.json | 1 + .../{custom-loader.js => custom-loader.cjs} | 0 .../documents/documents-from-glob.spec.ts | 2 +- .../tests/loaders/schema/integration.spec.ts | 2 +- packages/load/tests/use-custom-loader.node.ts | 17 +++++++++++++++++ packages/load/tests/use-custom-loader.spec.ts | 13 ++++++++----- 6 files changed, 28 insertions(+), 7 deletions(-) rename packages/load/tests/{custom-loader.js => custom-loader.cjs} (100%) create mode 100644 packages/load/tests/use-custom-loader.node.ts diff --git a/package.json b/package.json index 3a9c5a0e652..5e33bbf1ce5 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "test": "jest --no-watchman", "test-fed-compat": "ts-node --transpileOnly --compiler-options='{\"module\":\"commonjs\"}' scripts/fetch-federation-tests && yarn test federation-compat", "test:leaks": "cross-env \"LEAK_TEST=1\" jest --no-watchman --detectOpenHandles --detectLeaks --logHeapUsage", + "test:node": "node --experimental-strip-types --test **/tests/**/*.node.ts", "ts:check": "tsc --noEmit" }, "devDependencies": { diff --git a/packages/load/tests/custom-loader.js b/packages/load/tests/custom-loader.cjs similarity index 100% rename from packages/load/tests/custom-loader.js rename to packages/load/tests/custom-loader.cjs diff --git a/packages/load/tests/loaders/documents/documents-from-glob.spec.ts b/packages/load/tests/loaders/documents/documents-from-glob.spec.ts index 10696a1e88e..601fbde66bd 100644 --- a/packages/load/tests/loaders/documents/documents-from-glob.spec.ts +++ b/packages/load/tests/loaders/documents/documents-from-glob.spec.ts @@ -171,7 +171,7 @@ describe('documentsFromGlob', () => { loaderType: 'documents', }; const pointerOptions = { - loader: join(__dirname, '../../custom-loader.js'), + loader: join(__dirname, '../../custom-loader.cjs'), fooFieldName: 'myFooField', }; const result = await load( diff --git a/packages/load/tests/loaders/schema/integration.spec.ts b/packages/load/tests/loaders/schema/integration.spec.ts index a531a23f10d..e4d1380609d 100644 --- a/packages/load/tests/loaders/schema/integration.spec.ts +++ b/packages/load/tests/loaders/schema/integration.spec.ts @@ -150,7 +150,7 @@ describe('loadSchema', () => { loaderType: 'schema', }; const pointerOptions = { - loader: join(__dirname, '../../custom-loader.js'), + loader: join(__dirname, '../../custom-loader.cjs'), fooFieldName: 'myFooField', }; const result = await load( diff --git a/packages/load/tests/use-custom-loader.node.ts b/packages/load/tests/use-custom-loader.node.ts new file mode 100644 index 00000000000..cf050342c62 --- /dev/null +++ b/packages/load/tests/use-custom-loader.node.ts @@ -0,0 +1,17 @@ +import assert from 'node:assert'; +import { describe, it } from 'node:test'; +import { useCustomLoader } from '../src/utils/custom-loader.ts'; + +describe('useCustomLoader', () => { + it('can load a custom cjs loader from a file path', async () => { + const loader = await useCustomLoader(`./custom-loader.cjs`, import.meta.dirname); + const result = await loader('some-name', { customLoaderContext: {} }); + assert.strictEqual(result, 'I like turtles'); + }); + + it('can load a custom mjs loader from a file path', async () => { + const loader = await useCustomLoader(`./custom-loader.mjs`, import.meta.dirname); + const result = await loader('some-name', { customLoaderContext: {} }); + assert.strictEqual(result, 'I like turtles'); + }); +}); diff --git a/packages/load/tests/use-custom-loader.spec.ts b/packages/load/tests/use-custom-loader.spec.ts index a06d0f92fd0..9552e1977c9 100644 --- a/packages/load/tests/use-custom-loader.spec.ts +++ b/packages/load/tests/use-custom-loader.spec.ts @@ -1,9 +1,12 @@ import { useCustomLoader } from '../src/utils/custom-loader.js'; describe('useCustomLoader', () => { - it.each(['js', 'mjs'])('can load a custom loader from a file path', async (extension: string) => { - const loader = await useCustomLoader(`./custom-loader.${extension}`, __dirname); - const result = await loader('some-name', { customLoaderContext: {} }); - expect(result).toEqual('I like turtles'); - }); + it.each(['cjs', 'mjs'])( + 'can load a custom loader from a file path', + async (extension: string) => { + const loader = await useCustomLoader(`./custom-loader.${extension}`, __dirname); + const result = await loader('some-name', { customLoaderContext: {} }); + expect(result).toEqual('I like turtles'); + }, + ); }); From 542636d1eb421a30a1b773e9732d02cbe0486f24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Thu, 7 Nov 2024 23:05:40 +0000 Subject: [PATCH 3/8] use dynamic import to support custom esm loaders --- packages/load/src/utils/custom-loader.ts | 35 ++++++++++++------- packages/load/tests/use-custom-loader.node.ts | 8 ++++- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/load/src/utils/custom-loader.ts b/packages/load/src/utils/custom-loader.ts index 79f5c9435d9..0134b89c65d 100644 --- a/packages/load/src/utils/custom-loader.ts +++ b/packages/load/src/utils/custom-loader.ts @@ -1,20 +1,31 @@ import { createRequire } from 'module'; import { join as joinPaths } from 'path'; -export function getCustomLoaderByPath(path: string, cwd: string) { +function extractLoaderFromModule(loaderModule: any) { + if (loaderModule) { + if (loaderModule.default && typeof loaderModule.default === 'function') { + return loaderModule.default; + } + if (typeof loaderModule === 'function') { + return loaderModule; + } + } +} + +export async function getCustomLoaderByPath(path: string, cwd: string) { try { - const requireFn = createRequire(joinPaths(cwd, 'noop.js')); - const requiredModule = requireFn(path); + const importedModule = await import(joinPaths(cwd, path)); + return extractLoaderFromModule(importedModule); + } catch (e: any) {} - if (requiredModule) { - if (requiredModule.default && typeof requiredModule.default === 'function') { - return requiredModule.default; - } + return null; +} - if (typeof requiredModule === 'function') { - return requiredModule; - } - } +function getCustomLoaderByPathSync(path: string, cwd: string) { + try { + const requireFn = createRequire(joinPaths(cwd, 'noop.js')); + const requiredModule = requireFn(path); + return extractLoaderFromModule(requiredModule); } catch (e: any) {} return null; @@ -40,7 +51,7 @@ export function useCustomLoaderSync(loaderPointer: any, cwd: string) { let loader; if (typeof loaderPointer === 'string') { - loader = getCustomLoaderByPath(loaderPointer, cwd); + loader = getCustomLoaderByPathSync(loaderPointer, cwd); } else if (typeof loaderPointer === 'function') { loader = loaderPointer; } diff --git a/packages/load/tests/use-custom-loader.node.ts b/packages/load/tests/use-custom-loader.node.ts index cf050342c62..36658219557 100644 --- a/packages/load/tests/use-custom-loader.node.ts +++ b/packages/load/tests/use-custom-loader.node.ts @@ -1,6 +1,6 @@ import assert from 'node:assert'; import { describe, it } from 'node:test'; -import { useCustomLoader } from '../src/utils/custom-loader.ts'; +import { useCustomLoader, useCustomLoaderSync } from '../src/utils/custom-loader.ts'; describe('useCustomLoader', () => { it('can load a custom cjs loader from a file path', async () => { @@ -9,6 +9,12 @@ describe('useCustomLoader', () => { assert.strictEqual(result, 'I like turtles'); }); + it('can load a custom cjs loader synchronously', async () => { + const loader = useCustomLoaderSync(`./custom-loader.cjs`, import.meta.dirname); + const result = await loader('some-name', { customLoaderContext: {} }); + assert.strictEqual(result, 'I like turtles'); + }); + it('can load a custom mjs loader from a file path', async () => { const loader = await useCustomLoader(`./custom-loader.mjs`, import.meta.dirname); const result = await loader('some-name', { customLoaderContext: {} }); From 3c306426c27d9b88857e3cd5e7e3adb9bbde4fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Fri, 8 Nov 2024 09:14:08 +0000 Subject: [PATCH 4/8] Address ts import error by changing test from .ts to .mjs --- package.json | 2 +- .../{use-custom-loader.node.ts => use-custom-loader.node.mjs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/load/tests/{use-custom-loader.node.ts => use-custom-loader.node.mjs} (100%) diff --git a/package.json b/package.json index 5e33bbf1ce5..7507018ae3e 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test": "jest --no-watchman", "test-fed-compat": "ts-node --transpileOnly --compiler-options='{\"module\":\"commonjs\"}' scripts/fetch-federation-tests && yarn test federation-compat", "test:leaks": "cross-env \"LEAK_TEST=1\" jest --no-watchman --detectOpenHandles --detectLeaks --logHeapUsage", - "test:node": "node --experimental-strip-types --test **/tests/**/*.node.ts", + "test:node": "node --experimental-strip-types --test **/tests/**/*.node.*s", "ts:check": "tsc --noEmit" }, "devDependencies": { diff --git a/packages/load/tests/use-custom-loader.node.ts b/packages/load/tests/use-custom-loader.node.mjs similarity index 100% rename from packages/load/tests/use-custom-loader.node.ts rename to packages/load/tests/use-custom-loader.node.mjs From b0f6a3becdd08529cbd923257c0f6e394bc6cbcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Fri, 8 Nov 2024 10:13:50 +0000 Subject: [PATCH 5/8] Harden path building --- packages/load/src/utils/custom-loader.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/load/src/utils/custom-loader.ts b/packages/load/src/utils/custom-loader.ts index 0134b89c65d..984b2de60ea 100644 --- a/packages/load/src/utils/custom-loader.ts +++ b/packages/load/src/utils/custom-loader.ts @@ -1,5 +1,5 @@ import { createRequire } from 'module'; -import { join as joinPaths } from 'path'; +import { isAbsolute, join as joinPaths } from 'path'; function extractLoaderFromModule(loaderModule: any) { if (loaderModule) { @@ -14,7 +14,8 @@ function extractLoaderFromModule(loaderModule: any) { export async function getCustomLoaderByPath(path: string, cwd: string) { try { - const importedModule = await import(joinPaths(cwd, path)); + const absolutePath = isAbsolute(path) ? path : joinPaths(cwd, path); + const importedModule = await import(absolutePath); return extractLoaderFromModule(importedModule); } catch (e: any) {} From f3a2ff3f19e4ce1f9cf83be3c8f132a615ebcab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Fri, 8 Nov 2024 10:29:29 +0000 Subject: [PATCH 6/8] Run yarn test:node in CI on compatible node versions --- .github/workflows/tests.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a61bb617ced..11516ed033b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -110,6 +110,9 @@ jobs: timeout_minutes: 10 max_attempts: 5 command: yarn test:leaks --ci + - name: Tests using Node.js runner + if: ${{ matrix.node-version >= 22 }} + run: yarn test:node trackback: name: trackback rc dependencies @@ -158,7 +161,7 @@ jobs: with: timeout_minutes: 10 max_attempts: 5 - command: TEST_BROWSER=true yarn jest --no-watchman --ci browser + command: TEST_BROWSER=true yarn jest --no-watchman --ci browser analyze: name: Analyze runs-on: ubuntu-latest From 6e4d751eb78febe4a684e6c5107fc88bfc46052f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Fri, 8 Nov 2024 10:31:04 +0000 Subject: [PATCH 7/8] Revert (Jest) test back to original form and patch it up --- packages/load/tests/use-custom-loader.spec.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/load/tests/use-custom-loader.spec.ts b/packages/load/tests/use-custom-loader.spec.ts index 9552e1977c9..9da3c91f91a 100644 --- a/packages/load/tests/use-custom-loader.spec.ts +++ b/packages/load/tests/use-custom-loader.spec.ts @@ -1,12 +1,9 @@ -import { useCustomLoader } from '../src/utils/custom-loader.js'; +import { getCustomLoaderByPath } from '../src/utils/custom-loader.js'; -describe('useCustomLoader', () => { - it.each(['cjs', 'mjs'])( - 'can load a custom loader from a file path', - async (extension: string) => { - const loader = await useCustomLoader(`./custom-loader.${extension}`, __dirname); - const result = await loader('some-name', { customLoaderContext: {} }); - expect(result).toEqual('I like turtles'); - }, - ); +describe('getCustomLoaderByPath', () => { + it('can load a custom loader from a file path', async () => { + const loader = await getCustomLoaderByPath('./custom-loader.cjs', __dirname); + expect(loader).toBeDefined(); + expect(loader('some-name', { customLoaderContext: {} })).toEqual('I like turtles'); + }); }); From c4a1bf95d132192dd693f49d52e901c1464d80d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eir=C3=ADkur=20Torfason?= Date: Fri, 8 Nov 2024 11:20:07 +0000 Subject: [PATCH 8/8] Skip the CommonJS variants if using GraphQL 17 GraphQL 17 will only be bundled in ESM mode --- packages/load/tests/use-custom-loader.node.mjs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/load/tests/use-custom-loader.node.mjs b/packages/load/tests/use-custom-loader.node.mjs index 36658219557..2de785ee009 100644 --- a/packages/load/tests/use-custom-loader.node.mjs +++ b/packages/load/tests/use-custom-loader.node.mjs @@ -1,15 +1,18 @@ import assert from 'node:assert'; import { describe, it } from 'node:test'; +import { versionInfo as graphqlVersionInfo } from 'graphql'; import { useCustomLoader, useCustomLoaderSync } from '../src/utils/custom-loader.ts'; describe('useCustomLoader', () => { - it('can load a custom cjs loader from a file path', async () => { + const skipCjsTests = graphqlVersionInfo.major >= 17; + + it('can load a custom cjs loader from a file path', { skip: skipCjsTests }, async () => { const loader = await useCustomLoader(`./custom-loader.cjs`, import.meta.dirname); const result = await loader('some-name', { customLoaderContext: {} }); assert.strictEqual(result, 'I like turtles'); }); - it('can load a custom cjs loader synchronously', async () => { + it('can load a custom cjs loader synchronously', { skip: skipCjsTests }, async () => { const loader = useCustomLoaderSync(`./custom-loader.cjs`, import.meta.dirname); const result = await loader('some-name', { customLoaderContext: {} }); assert.strictEqual(result, 'I like turtles');