From ddd98e79e3e77be018e6f38fcea59e882d4806c8 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 27 Oct 2023 16:38:10 -0400 Subject: [PATCH 01/58] Stub out Package Importer tests, add changeEntryPoint to sandbox --- .../node-package-importer.node.test.ts | 150 ++++++++++++++++++ js-api-spec/sandbox.ts | 18 ++- 2 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 js-api-spec/node-package-importer.node.test.ts diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts new file mode 100644 index 0000000000..72a86f61a6 --- /dev/null +++ b/js-api-spec/node-package-importer.node.test.ts @@ -0,0 +1,150 @@ +// Copyright 2023 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import { + compile, + compileString, + compileStringAsync, + nodePackageImporter, + NodePackageImporter, +} from 'sass'; + +import {sandbox} from './sandbox'; + +function manifestBuilder(overrides?: {[key: string]: string}) { + return JSON.stringify({ + name: 'foo', + version: '1.0.0', + description: 'Verifying proposed pkg: loading algorithm', + scripts: { + test: 'echo "Error: no test specified" && exit 1', + }, + author: 'Package Author', + license: 'ISC', + ...overrides, + }); +} +xdescribe('resolves conditional exports', () => { + xit('sass at root'); + xit('sass with subpath'); + xit('style at root'); + xit('style with subpath'); + xit('style at root'); + xit('style with subpath'); +}); +describe('without subpath', () => { + it('sass key in package.json', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + sass: 'src/sass/_styles.scss', + }), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('style key in package.json', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + sass: 'src/sass/_styles.scss', + }), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + + ['index.scss', 'index.css', '_index.scss', '_index.css'].forEach(fileName => { + it(`loads from ${fileName}`, () => + sandbox(dir => { + dir.write({ + [`node_modules/foo/${fileName}`]: 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); + ['index.sass', '_index.sass'].forEach(fileName => { + it(`loads from ${fileName}`, () => + sandbox(dir => { + dir.write({ + [`node_modules/foo/${fileName}`]: 'a \n b: c', + 'node_modules/foo/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); +}); +xdescribe('with subpath', () => { + xit('resolves relative to package root'); +}); + +it('resolves from package', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); +xit('fake Node Package Importer', () => + sandbox(dir => { + dir.write({'foo/index.scss': 'a {from: dir}'}); + + const foo = {_NodePackageImporterBrand: ''} as NodePackageImporter; + + const result = compileString('@use "pkg:foo";', { + importers: [foo], + // loadPaths: [dir('dir')], + }); + expect(result.css).toBe('a {\n from: dir;\n}'); + })); +xdescribe('compilation methods', () => { + xit('compile'); + xit('compileString'); + xit('compileAsync'); + xit('compileStringAsync'); + xit('render'); + xit('renderSync'); +}); diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index 64a198fc11..a2a65c7fe1 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -49,13 +49,21 @@ export async function sandbox( fs.writeFileSync(fullPath, contents); } }, - chdir: (callback: () => unknown) => { + chdir: ( + callback: () => unknown, + options?: {changeEntryPoint: string} + ) => { const oldPath = process.cwd(); process.chdir(testDir); + const oldEntryPoint = require.main?.filename; + if (options?.changeEntryPoint) + require.main!.filename = `${testDir}/${options.changeEntryPoint}`; try { return callback(); } finally { process.chdir(oldPath); + if (options?.changeEntryPoint && oldEntryPoint) + require.main!.filename = oldEntryPoint; } }, }) @@ -93,6 +101,10 @@ interface SandboxDirectory { */ write(paths: {[path: string]: string}): void; - /** Runs `callback` with `root` as the current directory. */ - chdir(callback: () => T): void; + /** + * Runs `callback` with `root` as the current directory. If `changeEntryPoint` + * is set, moves the value of `require.main.filename` to a file relative to + * root. + * */ + chdir(callback: () => T, options?: {changeEntryPoint: string}): void; } From b489a76ad3b02a9afe4cdb580fee7d47601329c6 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Mon, 30 Oct 2023 09:54:06 -0400 Subject: [PATCH 02/58] More tests --- .../node-package-importer.node.test.ts | 155 +++++++++++++++--- js-api-spec/sandbox.ts | 2 +- 2 files changed, 137 insertions(+), 20 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 72a86f61a6..e701245c15 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -8,11 +8,16 @@ import { compileStringAsync, nodePackageImporter, NodePackageImporter, + FileImporter, } from 'sass'; -import {sandbox} from './sandbox'; +import {sandbox, SandboxDirectory} from './sandbox'; -function manifestBuilder(overrides?: {[key: string]: string}) { +const fileImporter = (dir: SandboxDirectory): FileImporter => ({ + findFileUrl: file => dir.url(file), +}); + +function manifestBuilder(overrides?: {[key: string]: string | Object}) { return JSON.stringify({ name: 'foo', version: '1.0.0', @@ -25,8 +30,33 @@ function manifestBuilder(overrides?: {[key: string]: string}) { ...overrides, }); } -xdescribe('resolves conditional exports', () => { - xit('sass at root'); +describe('resolves conditional exports', () => { + it('sass at root', () => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + '.': { + sass: './src/sass/_styles.scss', + import: './js/index.js', + require: './js/index.js', + }, + }, + }), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('aaa {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + xit('sass with subpath'); xit('style at root'); xit('style with subpath'); @@ -112,22 +142,109 @@ xdescribe('with subpath', () => { xit('resolves relative to package root'); }); -it('resolves from package', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), +describe('resolves from packages', () => { + it('resolves from entry point', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves from secondary @use', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_vendor.scss': '@use "pkg:bah";', + }); + dir.chdir( + () => { + const result = compileString('@use "vendor";', { + importers: [nodePackageImporter, fileImporter(dir)], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves from secondary @use pkg root', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves most proximate node_module', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bar/index.scss': 'e {f: g}', + 'node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + fit('resolves node_module above cwd', () => + sandbox(dir => { + dir.write({ + 'node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bar";', { + importers: [nodePackageImporter], + }); + return expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'deeply/nested/file/index.js'} + ); + })); + fit('throws if no match found', () => { + sandbox(dir => { + dir.chdir( + () => { + expect(() => + compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }) + ).toThrow(''); + }, + {changeEntryPoint: 'index.js'} + ); }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toBe('a {\n b: c;\n}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); + }); +}); xit('fake Node Package Importer', () => sandbox(dir => { dir.write({'foo/index.scss': 'a {from: dir}'}); diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index a2a65c7fe1..c10fb2dcaf 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -76,7 +76,7 @@ export async function sandbox( } /** The directory object passed to `sandbox`'s callback. */ -interface SandboxDirectory { +export interface SandboxDirectory { /** The root of the sandbox. */ readonly root: string; From 20ebaab0f3308a15b452a2caef55f2948f1fddde Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Mon, 30 Oct 2023 14:44:22 -0400 Subject: [PATCH 03/58] Parent directories --- .../node-package-importer.node.test.ts | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index e701245c15..573963c720 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -199,7 +199,7 @@ describe('resolves from packages', () => { dir.write({ 'node_modules/bah/index.scss': '@use "pkg:bar";', 'node_modules/bah/package.json': manifestBuilder(), - 'node_modules/bar/index.scss': 'e {f: g}', + 'node_modules/bar/index.scss': 'e {from: root}', 'node_modules/bar/package.json': manifestBuilder(), 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), @@ -214,7 +214,25 @@ describe('resolves from packages', () => { {changeEntryPoint: 'index.js'} ); })); - fit('resolves node_module above cwd', () => + it('resolves sub node_module', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves node_module above cwd', () => sandbox(dir => { dir.write({ 'node_modules/bar/index.scss': 'a {b: c}', @@ -230,7 +248,7 @@ describe('resolves from packages', () => { {changeEntryPoint: 'deeply/nested/file/index.js'} ); })); - fit('throws if no match found', () => { + it('throws if no match found', () => { sandbox(dir => { dir.chdir( () => { @@ -238,7 +256,9 @@ describe('resolves from packages', () => { compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }) - ).toThrow(''); + ).toThrowSassException({ + includes: "Node Package 'bah' could not be found", + }); }, {changeEntryPoint: 'index.js'} ); From 51214a62b45178260d2727f86bf084d9e1caea8b Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Mon, 30 Oct 2023 22:19:26 -0400 Subject: [PATCH 04/58] Add tests for all compilation methods --- .../node-package-importer.node.test.ts | 127 +++++++++++++++++- 1 file changed, 120 insertions(+), 7 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 573963c720..6a1897877d 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -4,10 +4,15 @@ import { compile, + compileAsync, compileString, compileStringAsync, + render, + renderSync, nodePackageImporter, NodePackageImporter, + LegacyException, + LegacyResult, FileImporter, } from 'sass'; @@ -277,11 +282,119 @@ xit('fake Node Package Importer', () => }); expect(result.css).toBe('a {\n from: dir;\n}'); })); -xdescribe('compilation methods', () => { - xit('compile'); - xit('compileString'); - xit('compileAsync'); - xit('compileStringAsync'); - xit('render'); - xit('renderSync'); +fdescribe('compilation methods', () => { + it('compile', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_index.scss': '@use "pkg:bah";', + }); + dir.chdir( + () => { + const result = compile('./_index.scss', { + importers: [nodePackageImporter, fileImporter(dir)], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('compileString', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + // TODO(jamesnw) This is a false positive + it('compileAsync', () => + sandbox(async dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_index.scss': '@use "pkg:bah";', + }); + dir.chdir( + async () => { + const result = await compileAsync('./_index.scss', { + importers: [nodePackageImporter, fileImporter(dir)], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toBe('a {\n b: SHOULD_FAIL;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('compileStringAsync', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + async () => { + const result = await compileStringAsync('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toBe('a {\n b: SHOULD_FAIL;\n}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('render', done => { + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + async () => { + await render( + { + data: '@import "pkg:bah"', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { sb: c; }' + ); + done(); + } + ); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it('renderSync', done => { + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = renderSync({ + data: '@import "pkg:bah"', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); }); From db1db07c26a9c47a9ea81f69d2d0c8cf9dd1d6a3 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 1 Nov 2023 10:27:17 -0400 Subject: [PATCH 05/58] Add package export tests --- .../node-package-importer.node.test.ts | 159 +++++++++++++++--- 1 file changed, 138 insertions(+), 21 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 6a1897877d..3217c210e5 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -35,38 +35,155 @@ function manifestBuilder(overrides?: {[key: string]: string | Object}) { ...overrides, }); } -describe('resolves conditional exports', () => { - it('sass at root', () => { + +function exportsBuilder(key: string) { + return { + exports: { + '.': { + [key]: './src/sass/_styles.scss', + }, + './colors/index.scss': { + [key]: './src/sass/colors/index.scss', + }, + './_variables.scss': { + [key]: './src/sass/_variables.scss', + }, + }, + }; +} + +fdescribe('resolves conditional exports', () => { + ['sass', 'style', 'default'].forEach(key => { + it(`${key} at root`, done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it(`${key} at root without non-pathed exports`, done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + [key]: './src/sass/_styles.scss', + }, + }), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + + xit(`${key} with subpath`, done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo/variables";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + xit(`${key} with index`, done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', + 'node_modules/foo/src/sass/colors/index.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo/colors";', { + importers: [nodePackageImporter], + }); + expect(result.css).toBe('a {\n b: c;\n}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + }); + it('throws if multiple paths found', done => { sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', 'node_modules/foo/package.json': manifestBuilder({ exports: { '.': { - sass: './src/sass/_styles.scss', - import: './js/index.js', - require: './js/index.js', + sass: './src/sass/_variables.scss', + style: './src/sass/_styles.scss', }, }, }), }); dir.chdir( () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toBe('aaa {\n b: c;\n}'); + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'Multiple resolutions'}); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it('resolves string export', done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: './src/sass/_variables.scss', + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + done(); }, {changeEntryPoint: 'index.js'} ); }); }); - - xit('sass with subpath'); - xit('style at root'); - xit('style with subpath'); - xit('style at root'); - xit('style with subpath'); }); describe('without subpath', () => { it('sass key in package.json', () => @@ -282,7 +399,7 @@ xit('fake Node Package Importer', () => }); expect(result.css).toBe('a {\n from: dir;\n}'); })); -fdescribe('compilation methods', () => { +describe('compilation methods', () => { it('compile', () => sandbox(dir => { dir.write({ @@ -359,10 +476,10 @@ fdescribe('compilation methods', () => { 'node_modules/bah/package.json': manifestBuilder(), }); dir.chdir( - async () => { - await render( + () => { + render( { - data: '@import "pkg:bah"', + data: '@use "pkg:bah"', pkgImporter: 'node', }, (err?: LegacyException, result?: LegacyResult) => { @@ -387,7 +504,7 @@ fdescribe('compilation methods', () => { dir.chdir( () => { const result = renderSync({ - data: '@import "pkg:bah"', + data: '@use "pkg:bah"', pkgImporter: 'node', }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); From 40f30ae737513655ed854e6da96059ba12555e81 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 2 Nov 2023 11:35:25 -0400 Subject: [PATCH 06/58] Tests for exports --- .../node-package-importer.node.test.ts | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 3217c210e5..4d6541ed54 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -52,7 +52,7 @@ function exportsBuilder(key: string) { }; } -fdescribe('resolves conditional exports', () => { +describe('resolves conditional exports', () => { ['sass', 'style', 'default'].forEach(key => { it(`${key} at root`, done => { sandbox(dir => { @@ -95,7 +95,7 @@ fdescribe('resolves conditional exports', () => { }); }); - xit(`${key} with subpath`, done => { + it(`${key} with subpath`, done => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -114,7 +114,7 @@ fdescribe('resolves conditional exports', () => { ); }); }); - xit(`${key} with index`, done => { + it(`${key} with index`, done => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -135,11 +135,11 @@ fdescribe('resolves conditional exports', () => { }); }); }); - it('throws if multiple paths found', done => { + it('compiles with first conditional match found', done => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', + 'node_modules/foo/src/sass/_variables.scss': 'a {from: sassCondition}', 'node_modules/foo/package.json': manifestBuilder({ exports: { '.': { @@ -149,13 +149,38 @@ fdescribe('resolves conditional exports', () => { }, }), }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it('throws if multiple exported paths match', done => { + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + './index.scss': {sass: './src/sass/_variables.scss'}, + './_index.sass': {sass: './src/sass/_styles.scss'}, + }, + }), + }); dir.chdir( () => { expect(() => compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }) - ).toThrowSassException({includes: 'Multiple resolutions'}); + ).toThrowSassException({includes: 'multiple potential resolutions'}); done(); }, {changeEntryPoint: 'index.js'} From f4748eba7ee3e53b7357f62921d23ccf1a345a79 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 2 Nov 2023 11:38:55 -0400 Subject: [PATCH 07/58] Clean up assertions --- .../node-package-importer.node.test.ts | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 4d6541ed54..fa39e172e2 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -65,7 +65,7 @@ describe('resolves conditional exports', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); done(); }, {changeEntryPoint: 'index.js'} @@ -87,7 +87,7 @@ describe('resolves conditional exports', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); done(); }, {changeEntryPoint: 'index.js'} @@ -107,7 +107,7 @@ describe('resolves conditional exports', () => { const result = compileString('@use "pkg:foo/variables";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); done(); }, {changeEntryPoint: 'index.js'} @@ -127,7 +127,7 @@ describe('resolves conditional exports', () => { const result = compileString('@use "pkg:foo/colors";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); done(); }, {changeEntryPoint: 'index.js'} @@ -224,7 +224,7 @@ describe('without subpath', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -242,7 +242,7 @@ describe('without subpath', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -260,7 +260,7 @@ describe('without subpath', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -278,7 +278,7 @@ describe('without subpath', () => { const result = compileString('@use "pkg:foo";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -301,7 +301,7 @@ describe('resolves from packages', () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -318,7 +318,7 @@ describe('resolves from packages', () => { const result = compileString('@use "vendor";', { importers: [nodePackageImporter, fileImporter(dir)], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -336,7 +336,7 @@ describe('resolves from packages', () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -356,7 +356,7 @@ describe('resolves from packages', () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -374,7 +374,7 @@ describe('resolves from packages', () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -390,7 +390,7 @@ describe('resolves from packages', () => { const result = compileString('@use "pkg:bar";', { importers: [nodePackageImporter], }); - return expect(result.css).toBe('a {\n b: c;\n}'); + return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'deeply/nested/file/index.js'} ); @@ -422,7 +422,7 @@ xit('fake Node Package Importer', () => importers: [foo], // loadPaths: [dir('dir')], }); - expect(result.css).toBe('a {\n from: dir;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a { from: dir;}'); })); describe('compilation methods', () => { it('compile', () => @@ -437,7 +437,7 @@ describe('compilation methods', () => { const result = compile('./_index.scss', { importers: [nodePackageImporter, fileImporter(dir)], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -453,7 +453,7 @@ describe('compilation methods', () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); @@ -471,8 +471,8 @@ describe('compilation methods', () => { const result = await compileAsync('./_index.scss', { importers: [nodePackageImporter, fileImporter(dir)], }); - expect(result.css).toBe('a {\n b: c;\n}'); - expect(result.css).toBe('a {\n b: SHOULD_FAIL;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); + expect(result.css).toEqualIgnoringWhitespace('a { b: SHOULD_FAIL;}'); }, {changeEntryPoint: 'index.js'} ); @@ -488,8 +488,8 @@ describe('compilation methods', () => { const result = await compileStringAsync('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toBe('a {\n b: c;\n}'); - expect(result.css).toBe('a {\n b: SHOULD_FAIL;\n}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + expect(result.css).toEqualIgnoringWhitespace('a {b: SHOULDFAIL;}'); }, {changeEntryPoint: 'index.js'} ); From 68eabbb513f436c0f472fd6ba19e4fbb2776db9b Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 2 Nov 2023 15:39:50 -0400 Subject: [PATCH 08/58] More legacy compilation tests --- .../node-package-importer.node.test.ts | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index fa39e172e2..780d3c1a76 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -458,7 +458,7 @@ describe('compilation methods', () => { {changeEntryPoint: 'index.js'} ); })); - // TODO(jamesnw) This is a false positive + // TODO(jamesnw) Async tests are returning false positives it('compileAsync', () => sandbox(async dir => { dir.write({ @@ -488,30 +488,58 @@ describe('compilation methods', () => { const result = await compileStringAsync('@use "pkg:bah";', { importers: [nodePackageImporter], }); + console.log(result.css, 'woo'); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); expect(result.css).toEqualIgnoringWhitespace('a {b: SHOULDFAIL;}'); }, {changeEntryPoint: 'index.js'} ); })); - it('render', done => { + it('render string', done => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), }); + dir.chdir( + async () => { + await render( + { + data: '@use "pkg:bah"', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + // expect(result!.css.toString()).toEqualIgnoringWhitespace( + // 'a { sb: c; }' + // ); + done(); + } + ); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it('render file', done => { + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + 'index.scss': '@use "pkg:bah";', + }); dir.chdir( () => { render( { - data: '@use "pkg:bah"', + file: 'index.scss', pkgImporter: 'node', }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { sb: c; }' - ); + // expect(result!.css.toString()).toEqualIgnoringWhitespace( + // 'a { sb: c; }' + // ); done(); } ); @@ -520,7 +548,27 @@ describe('compilation methods', () => { ); }); }); - it('renderSync', done => { + it('renderSync file', done => { + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + 'index.scss': '@use "pkg:bah";', + }); + dir.chdir( + () => { + const result = renderSync({ + file: 'index.scss', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + done(); + }, + {changeEntryPoint: 'index.js'} + ); + }); + }); + it('renderSync data', done => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', From 447ac0eafc42daebdf0f13262f5cea201caa2e5b Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 3 Nov 2023 10:47:15 -0400 Subject: [PATCH 09/58] Clean up false positives, return promise from sandbox test --- .../node-package-importer.node.test.ts | 84 ++++++++----------- js-api-spec/sandbox.ts | 2 +- 2 files changed, 34 insertions(+), 52 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 780d3c1a76..3ec154803f 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -54,7 +54,7 @@ function exportsBuilder(key: string) { describe('resolves conditional exports', () => { ['sass', 'style', 'default'].forEach(key => { - it(`${key} at root`, done => { + it(`${key} at root`, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', @@ -66,13 +66,11 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it(`${key} at root without non-pathed exports`, done => { + })); + it(`${key} at root without non-pathed exports`, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', @@ -88,14 +86,12 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); + })); - it(`${key} with subpath`, done => { + it(`${key} with subpath`, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -108,13 +104,11 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it(`${key} with index`, done => { + })); + it(`${key} with index`, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -128,14 +122,12 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); + })); }); - it('compiles with first conditional match found', done => { + it('compiles with first conditional match found', () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -156,13 +148,11 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }).css ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it('throws if multiple exported paths match', done => { + })); + it('throws if multiple exported paths match', () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -181,13 +171,11 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }) ).toThrowSassException({includes: 'multiple potential resolutions'}); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it('resolves string export', done => { + })); + it('resolves string export', () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', @@ -203,12 +191,10 @@ describe('resolves conditional exports', () => { importers: [nodePackageImporter], }).css ).toEqualIgnoringWhitespace('a {b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); + })); }); describe('without subpath', () => { it('sass key in package.json', () => @@ -458,21 +444,21 @@ describe('compilation methods', () => { {changeEntryPoint: 'index.js'} ); })); - // TODO(jamesnw) Async tests are returning false positives it('compileAsync', () => - sandbox(async dir => { + sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), '_index.scss': '@use "pkg:bah";', }); - dir.chdir( + return dir.chdir( async () => { const result = await compileAsync('./_index.scss', { importers: [nodePackageImporter, fileImporter(dir)], }); expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); expect(result.css).toEqualIgnoringWhitespace('a { b: SHOULD_FAIL;}'); + return result; }, {changeEntryPoint: 'index.js'} ); @@ -483,14 +469,14 @@ describe('compilation methods', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), }); - dir.chdir( + return dir.chdir( async () => { const result = await compileStringAsync('@use "pkg:bah";', { importers: [nodePackageImporter], }); - console.log(result.css, 'woo'); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); expect(result.css).toEqualIgnoringWhitespace('a {b: SHOULDFAIL;}'); + return result; }, {changeEntryPoint: 'index.js'} ); @@ -510,9 +496,9 @@ describe('compilation methods', () => { }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); - // expect(result!.css.toString()).toEqualIgnoringWhitespace( - // 'a { sb: c; }' - // ); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { sb: c; }' + ); done(); } ); @@ -529,17 +515,17 @@ describe('compilation methods', () => { 'index.scss': '@use "pkg:bah";', }); dir.chdir( - () => { - render( + async () => { + await render( { file: 'index.scss', pkgImporter: 'node', }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); - // expect(result!.css.toString()).toEqualIgnoringWhitespace( - // 'a { sb: c; }' - // ); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { sb: c; }' + ); done(); } ); @@ -548,43 +534,39 @@ describe('compilation methods', () => { ); }); }); - it('renderSync file', done => { + it('renderSync file', () => sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), 'index.scss': '@use "pkg:bah";', }); - dir.chdir( + return dir.chdir( () => { const result = renderSync({ file: 'index.scss', pkgImporter: 'node', }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it('renderSync data', done => { + })); + it('renderSync data', () => sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), }); - dir.chdir( + return dir.chdir( () => { const result = renderSync({ data: '@use "pkg:bah"', pkgImporter: 'node', }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - done(); }, {changeEntryPoint: 'index.js'} ); - }); - }); + })); }); diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index c10fb2dcaf..a26fa5dea3 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -32,7 +32,7 @@ export async function sandbox( ); } try { - await test( + return await test( Object.assign((...paths: string[]) => p.join(testDir, ...paths), { root: testDir, url: (...paths: string[]) => pathToFileURL(p.join(testDir, ...paths)), From 25500cbe9d6bc065e5a14617ab4235852912b046 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 3 Nov 2023 12:17:47 -0400 Subject: [PATCH 10/58] Promisify render tests to prevent sandbox dirs from being deleted before render called --- .../node-package-importer.node.test.ts | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 3ec154803f..175e46c37b 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -457,7 +457,6 @@ describe('compilation methods', () => { importers: [nodePackageImporter, fileImporter(dir)], }); expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); - expect(result.css).toEqualIgnoringWhitespace('a { b: SHOULD_FAIL;}'); return result; }, {changeEntryPoint: 'index.js'} @@ -475,65 +474,66 @@ describe('compilation methods', () => { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - expect(result.css).toEqualIgnoringWhitespace('a {b: SHOULDFAIL;}'); return result; }, {changeEntryPoint: 'index.js'} ); })); - it('render string', done => { + it('render string', () => sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), }); - dir.chdir( + return dir.chdir( async () => { - await render( - { - data: '@use "pkg:bah"', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { sb: c; }' - ); - done(); - } - ); + return await new Promise(resolve => { + render( + { + data: '@use "pkg:bah"', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); }, {changeEntryPoint: 'index.js'} ); - }); - }); - it('render file', done => { + })); + it('render file', () => sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': manifestBuilder(), 'index.scss': '@use "pkg:bah";', }); - dir.chdir( + return dir.chdir( async () => { - await render( - { - file: 'index.scss', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { sb: c; }' - ); - done(); - } - ); + return await new Promise(resolve => { + render( + { + file: 'index.scss', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); }, {changeEntryPoint: 'index.js'} ); - }); - }); + })); it('renderSync file', () => sandbox(dir => { dir.write({ From a024ff0f4562ab83827be36fca56812b3d239d71 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 3 Nov 2023 15:46:51 -0400 Subject: [PATCH 11/58] Nest Package Importer tests in describe --- .../node-package-importer.node.test.ts | 871 +++++++++--------- 1 file changed, 442 insertions(+), 429 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 175e46c37b..00c5a1fa9c 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -52,13 +52,168 @@ function exportsBuilder(key: string) { }; } -describe('resolves conditional exports', () => { - ['sass', 'style', 'default'].forEach(key => { - it(`${key} at root`, () => +describe('Node Package Importer', () => { + describe('resolves conditional exports', () => { + ['sass', 'style', 'default'].forEach(key => { + it(`${key} at root`, () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder( + exportsBuilder(key) + ), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it(`${key} at root without non-pathed exports`, () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + [key]: './src/sass/_styles.scss', + }, + }), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + + it(`${key} with subpath`, () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder( + exportsBuilder(key) + ), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo/variables";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it(`${key} with index`, () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', + 'node_modules/foo/src/sass/colors/index.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder( + exportsBuilder(key) + ), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo/colors";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); + it('compiles with first conditional match found', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': + 'a {from: sassCondition}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + '.': { + sass: './src/sass/_variables.scss', + style: './src/sass/_styles.scss', + }, + }, + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('throws if multiple exported paths match', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: { + './index.scss': {sass: './src/sass/_variables.scss'}, + './_index.sass': {sass: './src/sass/_styles.scss'}, + }, + }), + }); + dir.chdir( + () => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: 'multiple potential resolutions', + }); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves string export', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: './src/sass/_variables.scss', + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); + describe('without subpath', () => { + it('sass key in package.json', () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + 'node_modules/foo/package.json': manifestBuilder({ + sass: 'src/sass/_styles.scss', + }), }); dir.chdir( () => { @@ -70,14 +225,12 @@ describe('resolves conditional exports', () => { {changeEntryPoint: 'index.js'} ); })); - it(`${key} at root without non-pathed exports`, () => + it('style key in package.json', () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': manifestBuilder({ - exports: { - [key]: './src/sass/_styles.scss', - }, + sass: 'src/sass/_styles.scss', }), }); dir.chdir( @@ -91,16 +244,59 @@ describe('resolves conditional exports', () => { ); })); - it(`${key} with subpath`, () => + ['index.scss', 'index.css', '_index.scss', '_index.css'].forEach( + fileName => { + it(`loads from ${fileName}`, () => + sandbox(dir => { + dir.write({ + [`node_modules/foo/${fileName}`]: 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + } + ); + ['index.sass', '_index.sass'].forEach(fileName => { + it(`loads from ${fileName}`, () => + sandbox(dir => { + dir.write({ + [`node_modules/foo/${fileName}`]: 'a \n b: c', + 'node_modules/foo/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); + }); + xdescribe('with subpath', () => { + xit('resolves relative to package root'); + }); + + describe('resolves from packages', () => { + it('resolves from entry point', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), }); dir.chdir( () => { - const result = compileString('@use "pkg:foo/variables";', { + const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -108,160 +304,151 @@ describe('resolves conditional exports', () => { {changeEntryPoint: 'index.js'} ); })); - it(`${key} with index`, () => + it('resolves from secondary @use', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', - 'node_modules/foo/src/sass/colors/index.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder(exportsBuilder(key)), + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_vendor.scss': '@use "pkg:bah";', }); dir.chdir( () => { - const result = compileString('@use "pkg:foo/colors";', { - importers: [nodePackageImporter], + const result = compileString('@use "vendor";', { + importers: [nodePackageImporter, fileImporter(dir)], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); })); - }); - it('compiles with first conditional match found', () => - sandbox(dir => { - dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {from: sassCondition}', - 'node_modules/foo/package.json': manifestBuilder({ - exports: { - '.': { - sass: './src/sass/_variables.scss', - style: './src/sass/_styles.scss', - }, + it('resolves from secondary @use pkg root', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - }), - }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo";', { + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves most proximate node_module', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bar/index.scss': 'e {from: root}', + 'node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('throws if multiple exported paths match', () => - sandbox(dir => { - dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - exports: { - './index.scss': {sass: './src/sass/_variables.scss'}, - './_index.sass': {sass: './src/sass/_styles.scss'}, + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - }), - }); - dir.chdir( - () => { - expect(() => - compileString('@use "pkg:foo";', { + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves sub node_module', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], - }) - ).toThrowSassException({includes: 'multiple potential resolutions'}); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves string export', () => - sandbox(dir => { - dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - exports: './src/sass/_variables.scss', - }), - }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo";', { + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves node_module above cwd', () => + sandbox(dir => { + dir.write({ + 'node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bar";', { importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); -}); -describe('without subpath', () => { - it('sass key in package.json', () => - sandbox(dir => { - dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - sass: 'src/sass/_styles.scss', - }), + }); + return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'deeply/nested/file/index.js'} + ); + })); + it('throws if no match found', () => { + sandbox(dir => { + dir.chdir( + () => { + expect(() => + compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: "Node Package 'bah' could not be found", + }); + }, + {changeEntryPoint: 'index.js'} + ); }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('style key in package.json', () => + }); + }); + xit('fake Node Package Importer', () => sandbox(dir => { - dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - sass: 'src/sass/_styles.scss', - }), + dir.write({'foo/index.scss': 'a {from: dir}'}); + + const foo = {_NodePackageImporterBrand: ''} as NodePackageImporter; + + const result = compileString('@use "pkg:foo";', { + importers: [foo], + // loadPaths: [dir('dir')], }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + expect(result.css).toEqualIgnoringWhitespace('a { from: dir;}'); })); - - ['index.scss', 'index.css', '_index.scss', '_index.css'].forEach(fileName => { - it(`loads from ${fileName}`, () => + describe('compilation methods', () => { + it('compile', () => sandbox(dir => { dir.write({ - [`node_modules/foo/${fileName}`]: 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder(), + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_index.scss': '@use "pkg:bah";', }); dir.chdir( () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + const result = compile('./_index.scss', { + importers: [nodePackageImporter, fileImporter(dir)], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, {changeEntryPoint: 'index.js'} ); })); - }); - ['index.sass', '_index.sass'].forEach(fileName => { - it(`loads from ${fileName}`, () => + it('compileString', () => sandbox(dir => { dir.write({ - [`node_modules/foo/${fileName}`]: 'a \n b: c', - 'node_modules/foo/package.json': manifestBuilder(), + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), }); dir.chdir( () => { - const result = compileString('@use "pkg:foo";', { + const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -269,304 +456,130 @@ describe('without subpath', () => { {changeEntryPoint: 'index.js'} ); })); - }); -}); -xdescribe('with subpath', () => { - xit('resolves relative to package root'); -}); - -describe('resolves from packages', () => { - it('resolves from entry point', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves from secondary @use', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - '_vendor.scss': '@use "pkg:bah";', - }); - dir.chdir( - () => { - const result = compileString('@use "vendor";', { - importers: [nodePackageImporter, fileImporter(dir)], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves from secondary @use pkg root', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), - 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves most proximate node_module', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), - 'node_modules/bar/index.scss': 'e {from: root}', - 'node_modules/bar/package.json': manifestBuilder(), - 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves sub node_module', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), - 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('resolves node_module above cwd', () => - sandbox(dir => { - dir.write({ - 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bar";', { - importers: [nodePackageImporter], - }); - return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'deeply/nested/file/index.js'} - ); - })); - it('throws if no match found', () => { - sandbox(dir => { - dir.chdir( - () => { - expect(() => - compileString('@use "pkg:bah";', { + it('compileAsync', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + '_index.scss': '@use "pkg:bah";', + }); + return dir.chdir( + async () => { + const result = await compileAsync('./_index.scss', { + importers: [nodePackageImporter, fileImporter(dir)], + }); + expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); + return result; + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('compileStringAsync', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + return dir.chdir( + async () => { + const result = await compileStringAsync('@use "pkg:bah";', { importers: [nodePackageImporter], - }) - ).toThrowSassException({ - includes: "Node Package 'bah' could not be found", - }); - }, - {changeEntryPoint: 'index.js'} - ); - }); + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + return result; + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('render string', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + return dir.chdir( + async () => { + return await new Promise(resolve => { + render( + { + data: '@use "pkg:bah"', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('render file', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + 'index.scss': '@use "pkg:bah";', + }); + return dir.chdir( + async () => { + return await new Promise(resolve => { + render( + { + file: 'index.scss', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('renderSync file', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + 'index.scss': '@use "pkg:bah";', + }); + return dir.chdir( + () => { + const result = renderSync({ + file: 'index.scss', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('renderSync data', () => + sandbox(dir => { + dir.write({ + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': manifestBuilder(), + }); + return dir.chdir( + () => { + const result = renderSync({ + data: '@use "pkg:bah"', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); }); }); -xit('fake Node Package Importer', () => - sandbox(dir => { - dir.write({'foo/index.scss': 'a {from: dir}'}); - - const foo = {_NodePackageImporterBrand: ''} as NodePackageImporter; - - const result = compileString('@use "pkg:foo";', { - importers: [foo], - // loadPaths: [dir('dir')], - }); - expect(result.css).toEqualIgnoringWhitespace('a { from: dir;}'); - })); -describe('compilation methods', () => { - it('compile', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - '_index.scss': '@use "pkg:bah";', - }); - dir.chdir( - () => { - const result = compile('./_index.scss', { - importers: [nodePackageImporter, fileImporter(dir)], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('compileString', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('compileAsync', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - '_index.scss': '@use "pkg:bah";', - }); - return dir.chdir( - async () => { - const result = await compileAsync('./_index.scss', { - importers: [nodePackageImporter, fileImporter(dir)], - }); - expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); - return result; - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('compileStringAsync', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - return dir.chdir( - async () => { - const result = await compileStringAsync('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - return result; - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('render string', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - return dir.chdir( - async () => { - return await new Promise(resolve => { - render( - { - data: '@use "pkg:bah"', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { b: c; }' - ); - resolve(undefined); - } - ); - }); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('render file', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - 'index.scss': '@use "pkg:bah";', - }); - return dir.chdir( - async () => { - return await new Promise(resolve => { - render( - { - file: 'index.scss', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { b: c; }' - ); - resolve(undefined); - } - ); - }); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('renderSync file', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - 'index.scss': '@use "pkg:bah";', - }); - return dir.chdir( - () => { - const result = renderSync({ - file: 'index.scss', - pkgImporter: 'node', - }).css.toString(); - expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); - it('renderSync data', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - return dir.chdir( - () => { - const result = renderSync({ - data: '@use "pkg:bah"', - pkgImporter: 'node', - }).css.toString(); - expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); -}); From fad1cc35ee3cf24b8adcbeb90b0fc77b11c9a378 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Tue, 7 Nov 2023 16:37:25 -0500 Subject: [PATCH 12/58] Wildcard tests --- .../node-package-importer.node.test.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 00c5a1fa9c..9ef2f2e38a 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -205,6 +205,88 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'index.js'} ); })); + describe('wildcards', () => { + it('resolves with partial', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: {'./*.scss': './src/sass/*.scss'}, + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo/variables";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves file extension variant', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: {'./sass/*': './src/sass/*'}, + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo/sass/variables";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('resolves multipart paths', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: {'./*.scss': './src/*.scss'}, + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo/sass/variables";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); + it('throws if multiple wildcard export match', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/variables.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', + 'node_modules/foo/package.json': manifestBuilder({ + exports: {'./*.scss': './src/sass/*.scss'}, + }), + }); + dir.chdir( + () => { + expect( + () => + compileString('@use "pkg:foo/variables";', { + importers: [nodePackageImporter], + }).css + ).toThrowSassException({ + includes: 'multiple potential resolutions', + }); + }, + {changeEntryPoint: 'index.js'} + ); + })); + }); }); describe('without subpath', () => { it('sass key in package.json', () => From f5e413cda9f781c32087f8e7439efbb1c4e135b5 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 8 Nov 2023 12:24:08 -0500 Subject: [PATCH 13/58] Test legacy importer order --- js-api-spec/legacy/importer.node.test.ts | 32 +++++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/js-api-spec/legacy/importer.node.test.ts b/js-api-spec/legacy/importer.node.test.ts index 081d36f536..3d69de5dbb 100644 --- a/js-api-spec/legacy/importer.node.test.ts +++ b/js-api-spec/legacy/importer.node.test.ts @@ -35,7 +35,29 @@ it('an empty object means an empty file', () => describe('import precedence:', () => { describe('in sandbox dir', () => { - it('relative file is #1', () => + it('packageImporter:node is #1', () => + sandbox(dir => { + dir.write({ + 'sub/test.scss': 'a {from: relative}', + 'sub/node_modules/test/index.scss': 'a {from: pkg}', + 'sub/node_modules/test/package.json': '{}', + 'sub/base.scss': '@import "pkg:test"', + }); + + dir.chdir(() => + expect( + sass + .renderSync({ + file: dir('sub/base.scss'), + pkgImporter: 'node', + importer: () => ({contents: 'a {from: importer}'}), + }) + .css.toString() + ).toEqualIgnoringWhitespace('a { from: pkg; }') + ); + })); + + it('relative file is #2', () => sandbox(dir => { dir.write({ 'sub/test.scss': 'a {from: relative}', @@ -54,7 +76,7 @@ describe('import precedence:', () => { ); })); - it('importer is #2', () => + it('importer is #3', () => sandbox(dir => { dir.write({'test.scss': 'a {from: cwd}'}); @@ -63,14 +85,16 @@ describe('import precedence:', () => { sass .renderSync({ data: '@import "test"', - importer: () => ({contents: 'a {from: importer}'}), + importer: () => { + return {contents: 'a {from: importer}'}; + }, }) .css.toString() ).toEqualIgnoringWhitespace('a { from: importer; }') ); })); - it('CWD is #3', () => + it('CWD is #4', () => sandbox(dir => { dir.write({ 'test.scss': 'a {from: cwd}', From 1ddd7483fbd15204405411cb4cae12ca671bc3b2 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 8 Nov 2023 12:50:33 -0500 Subject: [PATCH 14/58] Flesh out stubbed tests --- .../node-package-importer.node.test.ts | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 9ef2f2e38a..b72c324c21 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -365,9 +365,23 @@ describe('Node Package Importer', () => { })); }); }); - xdescribe('with subpath', () => { - xit('resolves relative to package root'); - }); + + it('with subpath, resolves relative to package root', () => + sandbox(dir => { + dir.write({ + 'node_modules/bar/src/styles/sass/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': manifestBuilder(), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bar/src/styles/sass";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); describe('resolves from packages', () => { it('resolves from entry point', () => @@ -492,17 +506,17 @@ describe('Node Package Importer', () => { }); }); }); - xit('fake Node Package Importer', () => + it('faked Node Package Importer fails', () => sandbox(dir => { dir.write({'foo/index.scss': 'a {from: dir}'}); const foo = {_NodePackageImporterBrand: ''} as NodePackageImporter; - const result = compileString('@use "pkg:foo";', { - importers: [foo], - // loadPaths: [dir('dir')], - }); - expect(result.css).toEqualIgnoringWhitespace('a { from: dir;}'); + expect(() => + compileString('@use "pkg:foo";', { + importers: [foo], + }) + ).toThrow(); })); describe('compilation methods', () => { it('compile', () => From 3466dc64628fb02243b8d4fd61dcfb6a75c7c83a Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 8 Nov 2023 14:59:44 -0500 Subject: [PATCH 15/58] Change throw expectation --- js-api-spec/node-package-importer.node.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index b72c324c21..bf2ef3ef70 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -489,7 +489,7 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'deeply/nested/file/index.js'} ); })); - it('throws if no match found', () => { + it('continues if no match found', () => { sandbox(dir => { dir.chdir( () => { @@ -498,7 +498,7 @@ describe('Node Package Importer', () => { importers: [nodePackageImporter], }) ).toThrowSassException({ - includes: "Node Package 'bah' could not be found", + includes: "Can't find stylesheet to import", }); }, {changeEntryPoint: 'index.js'} From 5e04ad9ba27ca4fdf37fb26ac822fbdba5e6e00e Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 9 Nov 2023 15:45:43 -0500 Subject: [PATCH 16/58] Add browser-only test for no file system --- js-api-spec/importer.test.ts | 12 +++++++++++- js-api-spec/utils.ts | 7 +++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/js-api-spec/importer.test.ts b/js-api-spec/importer.test.ts index b632f90c25..9fcfb09da2 100644 --- a/js-api-spec/importer.test.ts +++ b/js-api-spec/importer.test.ts @@ -7,9 +7,10 @@ import { compileStringAsync, CanonicalizeContext, Importer, + nodePackageImporter, } from 'sass'; -import {sassImpl, URL} from './utils'; +import {sassImpl, skipForImpl, URL} from './utils'; it('uses an importer to resolve an @import', () => { const result = compileString('@import "orange";', { @@ -762,6 +763,15 @@ it('throws an ArgumentError when the result sourceMapUrl is missing a scheme', ( includes: 'Invalid argument (sourceMapUrl): must be absolute', }); }); +skipForImpl(['dart-sass', 'sass-embedded'], () => { + it('node package loader throws error in browser', () => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrow(); + }); +}); /** * Returns an importer that asserts that `fromImport` is `expected`, and diff --git a/js-api-spec/utils.ts b/js-api-spec/utils.ts index a75da896c3..28d2785f9d 100644 --- a/js-api-spec/utils.ts +++ b/js-api-spec/utils.ts @@ -12,12 +12,15 @@ export const isBrowser = !global.process; /** The name of the implementation of Sass being tested. */ export const sassImpl = info.split('\t')[0] as 'dart-sass' | 'sass-embedded'; +type Implementations = 'dart-sass' | 'sass-embedded' | 'browser'; + /** Skips the `block` of tests when running against the given `impl`. */ export function skipForImpl( - impl: 'dart-sass' | 'sass-embedded' | 'browser', + impl: Implementations | Implementations[], block: () => void ): void { - if (sassImpl === impl || (impl === 'browser' && isBrowser)) { + impl = Array.isArray(impl) ? impl : [impl]; + if (impl.includes(sassImpl) || (impl.includes('browser') && isBrowser)) { xdescribe(`[skipped for ${impl}]`, block); } else { block(); From 7b9743fb76f143257b1bc4a20da01a43f21b1db1 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 9 Nov 2023 16:15:32 -0500 Subject: [PATCH 17/58] Run test only in browser --- js-api-spec/importer.test.ts | 4 ++-- js-api-spec/utils.ts | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/js-api-spec/importer.test.ts b/js-api-spec/importer.test.ts index 9fcfb09da2..1693ca1ccc 100644 --- a/js-api-spec/importer.test.ts +++ b/js-api-spec/importer.test.ts @@ -10,7 +10,7 @@ import { nodePackageImporter, } from 'sass'; -import {sassImpl, skipForImpl, URL} from './utils'; +import {sassImpl, runOnlyForImpl, URL} from './utils'; it('uses an importer to resolve an @import', () => { const result = compileString('@import "orange";', { @@ -763,7 +763,7 @@ it('throws an ArgumentError when the result sourceMapUrl is missing a scheme', ( includes: 'Invalid argument (sourceMapUrl): must be absolute', }); }); -skipForImpl(['dart-sass', 'sass-embedded'], () => { +runOnlyForImpl('browser', () => { it('node package loader throws error in browser', () => { expect(() => compileString('@use "pkg:foo";', { diff --git a/js-api-spec/utils.ts b/js-api-spec/utils.ts index 28d2785f9d..29dabfa234 100644 --- a/js-api-spec/utils.ts +++ b/js-api-spec/utils.ts @@ -12,11 +12,11 @@ export const isBrowser = !global.process; /** The name of the implementation of Sass being tested. */ export const sassImpl = info.split('\t')[0] as 'dart-sass' | 'sass-embedded'; -type Implementations = 'dart-sass' | 'sass-embedded' | 'browser'; +type Implementation = 'dart-sass' | 'sass-embedded' | 'browser'; /** Skips the `block` of tests when running against the given `impl`. */ export function skipForImpl( - impl: Implementations | Implementations[], + impl: Implementation | Implementation[], block: () => void ): void { impl = Array.isArray(impl) ? impl : [impl]; @@ -27,6 +27,17 @@ export function skipForImpl( } } +export function runOnlyForImpl(impl: Implementation, block: () => void): void { + if ((impl === 'browser' && isBrowser) || impl === sassImpl) { + block(); + } else { + xdescribe( + `[skipped for ${sassImpl}${isBrowser ? ' in browser' : ''}]`, + block + ); + } +} + export const URL = isBrowser ? (global as unknown as any).URL : require('url').URL; From 83dc0a7ec8f1b8aefb64c72901c7dd6a2b852edd Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 15 Nov 2023 12:06:18 -0500 Subject: [PATCH 18/58] Review responses --- js-api-spec/importer.test.ts | 1 + js-api-spec/legacy/importer.node.test.ts | 28 +-- .../node-package-importer.node.test.ts | 225 ++++++++---------- js-api-spec/sandbox.ts | 3 +- 4 files changed, 107 insertions(+), 150 deletions(-) diff --git a/js-api-spec/importer.test.ts b/js-api-spec/importer.test.ts index 1693ca1ccc..8c9839e820 100644 --- a/js-api-spec/importer.test.ts +++ b/js-api-spec/importer.test.ts @@ -763,6 +763,7 @@ it('throws an ArgumentError when the result sourceMapUrl is missing a scheme', ( includes: 'Invalid argument (sourceMapUrl): must be absolute', }); }); + runOnlyForImpl('browser', () => { it('node package loader throws error in browser', () => { expect(() => diff --git a/js-api-spec/legacy/importer.node.test.ts b/js-api-spec/legacy/importer.node.test.ts index 3d69de5dbb..de2b68a480 100644 --- a/js-api-spec/legacy/importer.node.test.ts +++ b/js-api-spec/legacy/importer.node.test.ts @@ -35,29 +35,7 @@ it('an empty object means an empty file', () => describe('import precedence:', () => { describe('in sandbox dir', () => { - it('packageImporter:node is #1', () => - sandbox(dir => { - dir.write({ - 'sub/test.scss': 'a {from: relative}', - 'sub/node_modules/test/index.scss': 'a {from: pkg}', - 'sub/node_modules/test/package.json': '{}', - 'sub/base.scss': '@import "pkg:test"', - }); - - dir.chdir(() => - expect( - sass - .renderSync({ - file: dir('sub/base.scss'), - pkgImporter: 'node', - importer: () => ({contents: 'a {from: importer}'}), - }) - .css.toString() - ).toEqualIgnoringWhitespace('a { from: pkg; }') - ); - })); - - it('relative file is #2', () => + it('relative file is #1', () => sandbox(dir => { dir.write({ 'sub/test.scss': 'a {from: relative}', @@ -76,7 +54,7 @@ describe('import precedence:', () => { ); })); - it('importer is #3', () => + it('importer is #2', () => sandbox(dir => { dir.write({'test.scss': 'a {from: cwd}'}); @@ -94,7 +72,7 @@ describe('import precedence:', () => { ); })); - it('CWD is #4', () => + it('CWD is #3', () => sandbox(dir => { dir.write({ 'test.scss': 'a {from: cwd}', diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index bf2ef3ef70..b644ba8ccf 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -13,55 +13,24 @@ import { NodePackageImporter, LegacyException, LegacyResult, - FileImporter, } from 'sass'; -import {sandbox, SandboxDirectory} from './sandbox'; - -const fileImporter = (dir: SandboxDirectory): FileImporter => ({ - findFileUrl: file => dir.url(file), -}); - -function manifestBuilder(overrides?: {[key: string]: string | Object}) { - return JSON.stringify({ - name: 'foo', - version: '1.0.0', - description: 'Verifying proposed pkg: loading algorithm', - scripts: { - test: 'echo "Error: no test specified" && exit 1', - }, - author: 'Package Author', - license: 'ISC', - ...overrides, - }); -} - -function exportsBuilder(key: string) { - return { - exports: { - '.': { - [key]: './src/sass/_styles.scss', - }, - './colors/index.scss': { - [key]: './src/sass/colors/index.scss', - }, - './_variables.scss': { - [key]: './src/sass/_variables.scss', - }, - }, - }; -} +import {sandbox} from './sandbox'; describe('Node Package Importer', () => { describe('resolves conditional exports', () => { ['sass', 'style', 'default'].forEach(key => { - it(`${key} at root`, () => + it(`${key} for root `, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder( - exportsBuilder(key) - ), + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + '.': { + [key]: './src/sass/_styles.scss', + }, + }, + }), }); dir.chdir( () => { @@ -73,11 +42,11 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'index.js'} ); })); - it(`${key} at root without non-pathed exports`, () => + it(`${key} for root without '.'`, () => sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/package.json': JSON.stringify({ exports: { [key]: './src/sass/_styles.scss', }, @@ -97,15 +66,18 @@ describe('Node Package Importer', () => { it(`${key} with subpath`, () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder( - exportsBuilder(key) - ), + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + './_other.scss': { + [key]: './src/sass/_other.scss', + }, + }, + }), }); dir.chdir( () => { - const result = compileString('@use "pkg:foo/variables";', { + const result = compileString('@use "pkg:foo/other";', { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -117,15 +89,19 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'g {h: i}', - 'node_modules/foo/src/sass/colors/index.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder( - exportsBuilder(key) - ), + 'node_modules/foo/src/sass/_other.scss': 'g {h: i}', + 'node_modules/foo/src/sass/subdir/index.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + './subdir/index.scss': { + [key]: './src/sass/subdir/index.scss', + }, + }, + }), }); dir.chdir( () => { - const result = compileString('@use "pkg:foo/colors";', { + const result = compileString('@use "pkg:foo/subdir";', { importers: [nodePackageImporter], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -137,13 +113,12 @@ describe('Node Package Importer', () => { it('compiles with first conditional match found', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': - 'a {from: sassCondition}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/_styles.scss': 'd {from: styleCondition}', + 'node_modules/foo/src/sass/_other.scss': 'a {from: sassCondition}', + 'node_modules/foo/package.json': JSON.stringify({ exports: { '.': { - sass: './src/sass/_variables.scss', + sass: './src/sass/_other.scss', style: './src/sass/_styles.scss', }, }, @@ -164,10 +139,10 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ exports: { - './index.scss': {sass: './src/sass/_variables.scss'}, + './index.scss': {sass: './src/sass/_other.scss'}, './_index.sass': {sass: './src/sass/_styles.scss'}, }, }), @@ -189,9 +164,9 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - exports: './src/sass/_variables.scss', + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: './src/sass/_other.scss', }), }); dir.chdir( @@ -209,15 +184,15 @@ describe('Node Package Importer', () => { it('resolves with partial', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/sass/*.scss'}, }), }); dir.chdir( () => { expect( - compileString('@use "pkg:foo/variables";', { + compileString('@use "pkg:foo/other";', { importers: [nodePackageImporter], }).css ).toEqualIgnoringWhitespace('a {b: c;}'); @@ -228,15 +203,15 @@ describe('Node Package Importer', () => { it('resolves file extension variant', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ exports: {'./sass/*': './src/sass/*'}, }), }); dir.chdir( () => { expect( - compileString('@use "pkg:foo/sass/variables";', { + compileString('@use "pkg:foo/sass/other";', { importers: [nodePackageImporter], }).css ).toEqualIgnoringWhitespace('a {b: c;}'); @@ -247,15 +222,15 @@ describe('Node Package Importer', () => { it('resolves multipart paths', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/*.scss'}, }), }); dir.chdir( () => { expect( - compileString('@use "pkg:foo/sass/variables";', { + compileString('@use "pkg:foo/sass/other";', { importers: [nodePackageImporter], }).css ).toEqualIgnoringWhitespace('a {b: c;}'); @@ -263,12 +238,12 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'index.js'} ); })); - it('throws if multiple wildcard export match', () => + it('throws if multiple wildcard exports match', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/variables.scss': 'a {b: c}', - 'node_modules/foo/src/sass/_variables.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/src/sass/other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/sass/*.scss'}, }), }); @@ -276,7 +251,7 @@ describe('Node Package Importer', () => { () => { expect( () => - compileString('@use "pkg:foo/variables";', { + compileString('@use "pkg:foo/other";', { importers: [nodePackageImporter], }).css ).toThrowSassException({ @@ -293,7 +268,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ + 'node_modules/foo/package.json': JSON.stringify({ sass: 'src/sass/_styles.scss', }), }); @@ -311,8 +286,8 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder({ - sass: 'src/sass/_styles.scss', + 'node_modules/foo/package.json': JSON.stringify({ + style: 'src/sass/_styles.scss', }), }); dir.chdir( @@ -332,7 +307,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ [`node_modules/foo/${fileName}`]: 'a {b: c}', - 'node_modules/foo/package.json': manifestBuilder(), + 'node_modules/foo/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -351,7 +326,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ [`node_modules/foo/${fileName}`]: 'a \n b: c', - 'node_modules/foo/package.json': manifestBuilder(), + 'node_modules/foo/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -370,7 +345,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bar/src/styles/sass/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bar/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -384,33 +359,22 @@ describe('Node Package Importer', () => { })); describe('resolves from packages', () => { - it('resolves from entry point', () => - sandbox(dir => { - dir.write({ - 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), - }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); - })); it('resolves from secondary @use', () => sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), '_vendor.scss': '@use "pkg:bah";', }); dir.chdir( () => { const result = compileString('@use "vendor";', { - importers: [nodePackageImporter, fileImporter(dir)], + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, @@ -421,9 +385,9 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bar/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -439,18 +403,21 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bar/index.scss': 'e {from: root}', - 'node_modules/bar/package.json': manifestBuilder(), - 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar/index.scss': + 'a {from: submodule;}', + 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), }); dir.chdir( () => { const result = compileString('@use "pkg:bah";', { importers: [nodePackageImporter], }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + expect(result.css).toEqualIgnoringWhitespace( + 'a {from: submodule;}' + ); }, {changeEntryPoint: 'index.js'} ); @@ -459,9 +426,9 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': '@use "pkg:bar";', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -477,7 +444,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': manifestBuilder(), + 'node_modules/bar/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -489,7 +456,7 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'deeply/nested/file/index.js'} ); })); - it('continues if no match found', () => { + it('fails if no match found', () => { sandbox(dir => { dir.chdir( () => { @@ -523,13 +490,18 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), '_index.scss': '@use "pkg:bah";', }); dir.chdir( () => { const result = compile('./_index.scss', { - importers: [nodePackageImporter, fileImporter(dir)], + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, @@ -540,7 +512,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), }); dir.chdir( () => { @@ -556,13 +528,18 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), '_index.scss': '@use "pkg:bah";', }); return dir.chdir( async () => { const result = await compileAsync('./_index.scss', { - importers: [nodePackageImporter, fileImporter(dir)], + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], }); expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); return result; @@ -574,7 +551,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), }); return dir.chdir( async () => { @@ -591,7 +568,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), }); return dir.chdir( async () => { @@ -618,7 +595,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), 'index.scss': '@use "pkg:bah";', }); return dir.chdir( @@ -646,7 +623,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), 'index.scss': '@use "pkg:bah";', }); return dir.chdir( @@ -664,7 +641,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({ 'node_modules/bah/index.scss': 'a {b: c}', - 'node_modules/bah/package.json': manifestBuilder(), + 'node_modules/bah/package.json': JSON.stringify({}), }); return dir.chdir( () => { diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index a26fa5dea3..a6035257ea 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -56,8 +56,9 @@ export async function sandbox( const oldPath = process.cwd(); process.chdir(testDir); const oldEntryPoint = require.main?.filename; - if (options?.changeEntryPoint) + if (options?.changeEntryPoint) { require.main!.filename = `${testDir}/${options.changeEntryPoint}`; + } try { return callback(); } finally { From ccd2a7776797865cce733e3bac3aebe6b1541878 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 15 Nov 2023 12:09:12 -0500 Subject: [PATCH 19/58] Add full wildcard path test --- .../node-package-importer.node.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index b644ba8ccf..8b817345df 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -200,6 +200,25 @@ describe('Node Package Importer', () => { {changeEntryPoint: 'index.js'} ); })); + it('resolves with full wildcard path', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: {'./*': {sass: './src/sass/*'}}, + }), + }); + dir.chdir( + () => { + expect( + compileString('@use "pkg:foo/other";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {changeEntryPoint: 'index.js'} + ); + })); it('resolves file extension variant', () => sandbox(dir => { dir.write({ From b49db2c2e25a438b8865885ea2c4a0af5a4eeab3 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 15 Nov 2023 12:49:58 -0500 Subject: [PATCH 20/58] Move require.main.filename inside sandbox dir --- .../node-package-importer.node.test.ts | 568 ++++++++---------- js-api-spec/sandbox.ts | 22 +- 2 files changed, 248 insertions(+), 342 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 8b817345df..aea74e3356 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -32,15 +32,12 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it(`${key} for root without '.'`, () => sandbox(dir => { @@ -52,15 +49,12 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it(`${key} with subpath`, () => @@ -75,15 +69,12 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo/other";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it(`${key} with index`, () => sandbox(dir => { @@ -99,15 +90,12 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo/subdir";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo/subdir";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); }); it('compiles with first conditional match found', () => @@ -124,16 +112,13 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); + }); })); it('throws if multiple exported paths match', () => sandbox(dir => { @@ -147,18 +132,15 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir( - () => { - expect(() => - compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }) - ).toThrowSassException({ - includes: 'multiple potential resolutions', - }); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: 'multiple potential resolutions', + }); + }); })); it('resolves string export', () => sandbox(dir => { @@ -169,16 +151,13 @@ describe('Node Package Importer', () => { exports: './src/sass/_other.scss', }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); describe('wildcards', () => { it('resolves with partial', () => @@ -189,16 +168,13 @@ describe('Node Package Importer', () => { exports: {'./*.scss': './src/sass/*.scss'}, }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo/other";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves with full wildcard path', () => sandbox(dir => { @@ -208,16 +184,13 @@ describe('Node Package Importer', () => { exports: {'./*': {sass: './src/sass/*'}}, }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo/other";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves file extension variant', () => sandbox(dir => { @@ -227,16 +200,13 @@ describe('Node Package Importer', () => { exports: {'./sass/*': './src/sass/*'}, }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo/sass/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo/sass/other";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves multipart paths', () => sandbox(dir => { @@ -246,16 +216,13 @@ describe('Node Package Importer', () => { exports: {'./*.scss': './src/*.scss'}, }), }); - dir.chdir( - () => { - expect( - compileString('@use "pkg:foo/sass/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + compileString('@use "pkg:foo/sass/other";', { + importers: [nodePackageImporter], + }).css + ).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('throws if multiple wildcard exports match', () => sandbox(dir => { @@ -266,19 +233,16 @@ describe('Node Package Importer', () => { exports: {'./*.scss': './src/sass/*.scss'}, }), }); - dir.chdir( - () => { - expect( - () => - compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }).css - ).toThrowSassException({ - includes: 'multiple potential resolutions', - }); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect( + () => + compileString('@use "pkg:foo/other";', { + importers: [nodePackageImporter], + }).css + ).toThrowSassException({ + includes: 'multiple potential resolutions', + }); + }); })); }); }); @@ -291,15 +255,12 @@ describe('Node Package Importer', () => { sass: 'src/sass/_styles.scss', }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('style key in package.json', () => sandbox(dir => { @@ -309,15 +270,12 @@ describe('Node Package Importer', () => { style: 'src/sass/_styles.scss', }), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); ['index.scss', 'index.css', '_index.scss', '_index.css'].forEach( @@ -328,15 +286,12 @@ describe('Node Package Importer', () => { [`node_modules/foo/${fileName}`]: 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); } ); @@ -347,15 +302,12 @@ describe('Node Package Importer', () => { [`node_modules/foo/${fileName}`]: 'a \n b: c', 'node_modules/foo/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); }); }); @@ -366,15 +318,12 @@ describe('Node Package Importer', () => { 'node_modules/bar/src/styles/sass/index.scss': 'a {b: c}', 'node_modules/bar/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bar/src/styles/sass";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:bar/src/styles/sass";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); describe('resolves from packages', () => { @@ -385,20 +334,17 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), '_vendor.scss': '@use "pkg:bah";', }); - dir.chdir( - () => { - const result = compileString('@use "vendor";', { - importers: [ - nodePackageImporter, - { - findFileUrl: file => dir.url(file), - }, - ], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "vendor";', { + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves from secondary @use pkg root', () => sandbox(dir => { @@ -408,15 +354,12 @@ describe('Node Package Importer', () => { 'node_modules/bar/index.scss': 'a {b: c}', 'node_modules/bar/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves most proximate node_module', () => sandbox(dir => { @@ -429,17 +372,12 @@ describe('Node Package Importer', () => { 'a {from: submodule;}', 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace( - 'a {from: submodule;}' - ); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {from: submodule;}'); + }); })); it('resolves sub node_module', () => sandbox(dir => { @@ -449,15 +387,12 @@ describe('Node Package Importer', () => { 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves node_module above cwd', () => sandbox(dir => { @@ -472,23 +407,20 @@ describe('Node Package Importer', () => { }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - {changeEntryPoint: 'deeply/nested/file/index.js'} + {entryPoint: 'deeply/nested/file/index.js'} ); })); it('fails if no match found', () => { sandbox(dir => { - dir.chdir( - () => { - expect(() => - compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }) - ).toThrowSassException({ - includes: "Can't find stylesheet to import", - }); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + expect(() => + compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: "Can't find stylesheet to import", + }); + }); }); }); }); @@ -512,20 +444,17 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), '_index.scss': '@use "pkg:bah";', }); - dir.chdir( - () => { - const result = compile('./_index.scss', { - importers: [ - nodePackageImporter, - { - findFileUrl: file => dir.url(file), - }, - ], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compile('./_index.scss', { + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('compileString', () => sandbox(dir => { @@ -533,15 +462,12 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - dir.chdir( - () => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('compileAsync', () => sandbox(dir => { @@ -550,21 +476,18 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), '_index.scss': '@use "pkg:bah";', }); - return dir.chdir( - async () => { - const result = await compileAsync('./_index.scss', { - importers: [ - nodePackageImporter, - { - findFileUrl: file => dir.url(file), - }, - ], - }); - expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); - return result; - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(async () => { + const result = await compileAsync('./_index.scss', { + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], + }); + expect(result.css).toEqualIgnoringWhitespace('a { b: c;}'); + return result; + }); })); it('compileStringAsync', () => sandbox(dir => { @@ -572,16 +495,13 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - return dir.chdir( - async () => { - const result = await compileStringAsync('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - return result; - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(async () => { + const result = await compileStringAsync('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + return result; + }); })); it('render string', () => sandbox(dir => { @@ -589,26 +509,23 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - return dir.chdir( - async () => { - return await new Promise(resolve => { - render( - { - data: '@use "pkg:bah"', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { b: c; }' - ); - resolve(undefined); - } - ); - }); - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(async () => { + return await new Promise(resolve => { + render( + { + data: '@use "pkg:bah"', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); + }); })); it('render file', () => sandbox(dir => { @@ -617,26 +534,23 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), 'index.scss': '@use "pkg:bah";', }); - return dir.chdir( - async () => { - return await new Promise(resolve => { - render( - { - file: 'index.scss', - pkgImporter: 'node', - }, - (err?: LegacyException, result?: LegacyResult) => { - expect(err).toBeFalsy(); - expect(result!.css.toString()).toEqualIgnoringWhitespace( - 'a { b: c; }' - ); - resolve(undefined); - } - ); - }); - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(async () => { + return await new Promise(resolve => { + render( + { + file: 'index.scss', + pkgImporter: 'node', + }, + (err?: LegacyException, result?: LegacyResult) => { + expect(err).toBeFalsy(); + expect(result!.css.toString()).toEqualIgnoringWhitespace( + 'a { b: c; }' + ); + resolve(undefined); + } + ); + }); + }); })); it('renderSync file', () => sandbox(dir => { @@ -645,16 +559,13 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), 'index.scss': '@use "pkg:bah";', }); - return dir.chdir( - () => { - const result = renderSync({ - file: 'index.scss', - pkgImporter: 'node', - }).css.toString(); - expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(() => { + const result = renderSync({ + file: 'index.scss', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + }); })); it('renderSync data', () => sandbox(dir => { @@ -662,16 +573,13 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - return dir.chdir( - () => { - const result = renderSync({ - data: '@use "pkg:bah"', - pkgImporter: 'node', - }).css.toString(); - expect(result).toEqualIgnoringWhitespace('a { b: c;}'); - }, - {changeEntryPoint: 'index.js'} - ); + return dir.chdir(() => { + const result = renderSync({ + data: '@use "pkg:bah"', + pkgImporter: 'node', + }).css.toString(); + expect(result).toEqualIgnoringWhitespace('a { b: c;}'); + }); })); }); }); diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index a6035257ea..0a542e5a55 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -49,22 +49,19 @@ export async function sandbox( fs.writeFileSync(fullPath, contents); } }, - chdir: ( - callback: () => unknown, - options?: {changeEntryPoint: string} - ) => { + chdir: (callback: () => unknown, options?: {entryPoint: string}) => { const oldPath = process.cwd(); process.chdir(testDir); const oldEntryPoint = require.main?.filename; - if (options?.changeEntryPoint) { - require.main!.filename = `${testDir}/${options.changeEntryPoint}`; + if (oldEntryPoint) { + const filename = options?.entryPoint || p.basename(oldEntryPoint); + require.main!.filename = `${testDir}/${filename}`; } try { return callback(); } finally { process.chdir(oldPath); - if (options?.changeEntryPoint && oldEntryPoint) - require.main!.filename = oldEntryPoint; + if (oldEntryPoint) require.main!.filename = oldEntryPoint; } }, }) @@ -103,9 +100,10 @@ export interface SandboxDirectory { write(paths: {[path: string]: string}): void; /** - * Runs `callback` with `root` as the current directory. If `changeEntryPoint` - * is set, moves the value of `require.main.filename` to a file relative to - * root. + * Runs `callback` with `root` as the current directory, and moves + * `require.main.filename` into `root`. If `entryPoint` is set, it uses that + * as the filename within the directory, otherwise it uses the basename of the + * original `require.main.filename`. * */ - chdir(callback: () => T, options?: {changeEntryPoint: string}): void; + chdir(callback: () => T, options?: {entryPoint: string}): void; } From f96ba8dea03b2bd0197cde036582c3df1dbbd09e Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 15 Nov 2023 13:44:44 -0500 Subject: [PATCH 21/58] Factor out common testPackageImporter logic --- .../node-package-importer.node.test.ts | 293 ++++++++---------- 1 file changed, 125 insertions(+), 168 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index aea74e3356..30b6c103d4 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -17,12 +17,34 @@ import { import {sandbox} from './sandbox'; +const testPackageImporter = ({ + input, + output, + files, +}: { + input: string; + output: string; + files: {[path: string]: string}; +}) => { + sandbox(dir => { + dir.write(files); + dir.chdir(() => { + const result = compileString(input, { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace(output); + }); + }); +}; + describe('Node Package Importer', () => { describe('resolves conditional exports', () => { ['sass', 'style', 'default'].forEach(key => { it(`${key} for root `, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: { @@ -31,35 +53,28 @@ describe('Node Package Importer', () => { }, }, }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it(`${key} for root without '.'`, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: { [key]: './src/sass/_styles.scss', }, }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); it(`${key} with subpath`, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/other";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: { @@ -68,17 +83,14 @@ describe('Node Package Importer', () => { }, }, }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it(`${key} with index`, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/subdir";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', 'node_modules/foo/src/sass/_other.scss': 'g {h: i}', 'node_modules/foo/src/sass/subdir/index.scss': 'a {b: c}', @@ -89,18 +101,15 @@ describe('Node Package Importer', () => { }, }, }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo/subdir";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); }); + it('compiles with first conditional match found', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {from: sassCondition;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'd {from: styleCondition}', 'node_modules/foo/src/sass/_other.scss': 'a {from: sassCondition}', 'node_modules/foo/package.json': JSON.stringify({ @@ -111,15 +120,9 @@ describe('Node Package Importer', () => { }, }, }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {from: sassCondition;}'); - }); + }, })); + it('throws if multiple exported paths match', () => sandbox(dir => { dir.write({ @@ -142,88 +145,69 @@ describe('Node Package Importer', () => { }); }); })); + it('resolves string export', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: './src/sass/_other.scss', }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + describe('wildcards', () => { it('resolves with partial', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/other";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/sass/*.scss'}, }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('resolves with full wildcard path', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/other";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*': {sass: './src/sass/*'}}, }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('resolves file extension variant', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/sass/other";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./sass/*': './src/sass/*'}, }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo/sass/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('resolves multipart paths', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo/sass/other";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/*.scss'}, }), - }); - dir.chdir(() => { - expect( - compileString('@use "pkg:foo/sass/other";', { - importers: [nodePackageImporter], - }).css - ).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('throws if multiple wildcard exports match', () => sandbox(dir => { dir.write({ @@ -246,84 +230,66 @@ describe('Node Package Importer', () => { })); }); }); + describe('without subpath', () => { it('sass key in package.json', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ sass: 'src/sass/_styles.scss', }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('style key in package.json', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ style: 'src/sass/_styles.scss', }), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); ['index.scss', 'index.css', '_index.scss', '_index.css'].forEach( fileName => { it(`loads from ${fileName}`, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { [`node_modules/foo/${fileName}`]: 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); } ); ['index.sass', '_index.sass'].forEach(fileName => { it(`loads from ${fileName}`, () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {b: c;}', + files: { [`node_modules/foo/${fileName}`]: 'a \n b: c', 'node_modules/foo/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); }); }); it('with subpath, resolves relative to package root', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:bar/src/styles/sass";', + output: 'a {b: c;}', + files: { 'node_modules/bar/src/styles/sass/index.scss': 'a {b: c}', 'node_modules/bar/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:bar/src/styles/sass";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); describe('resolves from packages', () => { @@ -347,23 +313,22 @@ describe('Node Package Importer', () => { }); })); it('resolves from secondary @use pkg root', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:bah";', + output: 'a {b: c;}', + files: { 'node_modules/bah/index.scss': '@use "pkg:bar";', 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bar/index.scss': 'a {b: c}', 'node_modules/bar/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('resolves most proximate node_module', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:bah";', + output: 'a {from: submodule;}', + files: { 'node_modules/bah/index.scss': '@use "pkg:bar";', 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bar/index.scss': 'e {from: root}', @@ -371,29 +336,21 @@ describe('Node Package Importer', () => { 'node_modules/bah/node_modules/bar/index.scss': 'a {from: submodule;}', 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {from: submodule;}'); - }); + }, })); + it('resolves sub node_module', () => - sandbox(dir => { - dir.write({ + testPackageImporter({ + input: '@use "pkg:bah";', + output: 'a {b: c;}', + files: { 'node_modules/bah/index.scss': '@use "pkg:bar";', 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), - }); - dir.chdir(() => { - const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], - }); - expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }); + }, })); + it('resolves node_module above cwd', () => sandbox(dir => { dir.write({ From 88d5ae670f071a5cde4e33db6c39d739c7b3e44a Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 15 Nov 2023 15:04:59 -0500 Subject: [PATCH 22/58] Fix testPackageImporter call, add scoped packages and relative import tests --- .../node-package-importer.node.test.ts | 44 +++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 30b6c103d4..50f721c614 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -25,7 +25,7 @@ const testPackageImporter = ({ input: string; output: string; files: {[path: string]: string}; -}) => { +}) => sandbox(dir => { dir.write(files); dir.chdir(() => { @@ -35,7 +35,6 @@ const testPackageImporter = ({ expect(result.css).toEqualIgnoringWhitespace(output); }); }); -}; describe('Node Package Importer', () => { describe('resolves conditional exports', () => { @@ -151,7 +150,6 @@ describe('Node Package Importer', () => { input: '@use "pkg:foo";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: './src/sass/_other.scss', @@ -312,6 +310,7 @@ describe('Node Package Importer', () => { expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }); })); + it('resolves from secondary @use pkg root', () => testPackageImporter({ input: '@use "pkg:bah";', @@ -324,6 +323,19 @@ describe('Node Package Importer', () => { }, })); + it('relative import in package', () => + testPackageImporter({ + input: '@use "pkg:foo";', + output: 'a {db: c;}', + files: { + 'node_modules/foo/scss/foo.scss': '@use "mixins/banner";', + 'node_modules/foo/scss/mixins/_banner.scss': 'a {b: c;}', + 'node_modules/foo/package.json': JSON.stringify({ + sass: 'scss/foo.scss', + }), + }, + })); + it('resolves most proximate node_module', () => testPackageImporter({ input: '@use "pkg:bah";', @@ -367,6 +379,32 @@ describe('Node Package Importer', () => { {entryPoint: 'deeply/nested/file/index.js'} ); })); + + it('resolves in scoped package', () => + testPackageImporter({ + input: '@use "pkg:@foo/bar";', + output: 'a {b: c;}', + files: { + 'node_modules/@foo/bar/src/sass/_styles.scss': 'd {e: f}', + 'node_modules/@foo/bar/src/sass/_other.scss': 'a {b: c}', + 'node_modules/@foo/bar/package.json': JSON.stringify({ + exports: './src/sass/_other.scss', + }), + }, + })); + + it('resolves from secondary @use in scoped packages', () => + testPackageImporter({ + input: '@use "pkg:@foo/bah";', + output: 'a {b: c;}', + files: { + 'node_modules/@foo/bah/index.scss': '@use "pkg:@foo/bar";', + 'node_modules/@foo/bah/package.json': JSON.stringify({}), + 'node_modules/@foo/bar/index.scss': 'a {b: c}', + 'node_modules/@foo/bar/package.json': JSON.stringify({}), + }, + })); + it('fails if no match found', () => { sandbox(dir => { dir.chdir(() => { From 551b5b758c7e595b89635fba372dd013af447e97 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 16 Nov 2023 10:50:41 -0500 Subject: [PATCH 23/58] Clean up paths --- .../node-package-importer.node.test.ts | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 50f721c614..5539721228 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -71,14 +71,14 @@ describe('Node Package Importer', () => { it(`${key} with subpath`, () => testPackageImporter({ - input: '@use "pkg:foo/other";', + input: '@use "pkg:foo/styles";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: { - './_other.scss': { - [key]: './src/sass/_other.scss', + './_styles.scss': { + [key]: './src/sass/_styles.scss', }, }, }), @@ -90,8 +90,6 @@ describe('Node Package Importer', () => { input: '@use "pkg:foo/subdir";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/foo/src/sass/_other.scss': 'g {h: i}', 'node_modules/foo/src/sass/subdir/index.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: { @@ -110,11 +108,11 @@ describe('Node Package Importer', () => { output: 'a {from: sassCondition;}', files: { 'node_modules/foo/src/sass/_styles.scss': 'd {from: styleCondition}', - 'node_modules/foo/src/sass/_other.scss': 'a {from: sassCondition}', + 'node_modules/foo/src/sass/_sass.scss': 'a {from: sassCondition}', 'node_modules/foo/package.json': JSON.stringify({ exports: { '.': { - sass: './src/sass/_other.scss', + sass: './src/sass/_sass.scss', style: './src/sass/_styles.scss', }, }, @@ -150,9 +148,9 @@ describe('Node Package Importer', () => { input: '@use "pkg:foo";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ - exports: './src/sass/_other.scss', + exports: './src/sass/_styles.scss', }), }, })); @@ -160,22 +158,22 @@ describe('Node Package Importer', () => { describe('wildcards', () => { it('resolves with partial', () => testPackageImporter({ - input: '@use "pkg:foo/other";', + input: '@use "pkg:foo/styles";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/sass/*.scss'}, }), }, })); - it('resolves with full wildcard path', () => + it('resolves with full wildcard path and sass conditional export', () => testPackageImporter({ - input: '@use "pkg:foo/other";', + input: '@use "pkg:foo/styles";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*': {sass: './src/sass/*'}}, }), @@ -184,10 +182,10 @@ describe('Node Package Importer', () => { it('resolves file extension variant', () => testPackageImporter({ - input: '@use "pkg:foo/sass/other";', + input: '@use "pkg:foo/sass/styles";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./sass/*': './src/sass/*'}, }), @@ -196,10 +194,10 @@ describe('Node Package Importer', () => { it('resolves multipart paths', () => testPackageImporter({ - input: '@use "pkg:foo/sass/other";', + input: '@use "pkg:foo/sass/styles";', output: 'a {b: c;}', files: { - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/*.scss'}, }), @@ -209,8 +207,8 @@ describe('Node Package Importer', () => { it('throws if multiple wildcard exports match', () => sandbox(dir => { dir.write({ - 'node_modules/foo/src/sass/other.scss': 'a {b: c}', - 'node_modules/foo/src/sass/_other.scss': 'a {b: c}', + 'node_modules/foo/src/sass/styles.scss': 'a {b: c}', + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', 'node_modules/foo/package.json': JSON.stringify({ exports: {'./*.scss': './src/sass/*.scss'}, }), @@ -218,7 +216,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect( () => - compileString('@use "pkg:foo/other";', { + compileString('@use "pkg:foo/styles";', { importers: [nodePackageImporter], }).css ).toThrowSassException({ @@ -328,10 +326,10 @@ describe('Node Package Importer', () => { input: '@use "pkg:foo";', output: 'a {db: c;}', files: { - 'node_modules/foo/scss/foo.scss': '@use "mixins/banner";', + 'node_modules/foo/scss/styles.scss': '@use "mixins/banner";', 'node_modules/foo/scss/mixins/_banner.scss': 'a {b: c;}', 'node_modules/foo/package.json': JSON.stringify({ - sass: 'scss/foo.scss', + sass: 'scss/styles.scss', }), }, })); @@ -385,10 +383,9 @@ describe('Node Package Importer', () => { input: '@use "pkg:@foo/bar";', output: 'a {b: c;}', files: { - 'node_modules/@foo/bar/src/sass/_styles.scss': 'd {e: f}', - 'node_modules/@foo/bar/src/sass/_other.scss': 'a {b: c}', + 'node_modules/@foo/bar/src/sass/_styles.scss': 'a {b: c}', 'node_modules/@foo/bar/package.json': JSON.stringify({ - exports: './src/sass/_other.scss', + exports: './src/sass/_styles.scss', }), }, })); @@ -419,6 +416,7 @@ describe('Node Package Importer', () => { }); }); }); + it('faked Node Package Importer fails', () => sandbox(dir => { dir.write({'foo/index.scss': 'a {from: dir}'}); @@ -431,6 +429,7 @@ describe('Node Package Importer', () => { }) ).toThrow(); })); + describe('compilation methods', () => { it('compile', () => sandbox(dir => { @@ -451,6 +450,7 @@ describe('Node Package Importer', () => { expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }); })); + it('compileString', () => sandbox(dir => { dir.write({ @@ -464,6 +464,7 @@ describe('Node Package Importer', () => { expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }); })); + it('compileAsync', () => sandbox(dir => { dir.write({ @@ -484,6 +485,7 @@ describe('Node Package Importer', () => { return result; }); })); + it('compileStringAsync', () => sandbox(dir => { dir.write({ @@ -498,6 +500,7 @@ describe('Node Package Importer', () => { return result; }); })); + it('render string', () => sandbox(dir => { dir.write({ @@ -522,6 +525,7 @@ describe('Node Package Importer', () => { }); }); })); + it('render file', () => sandbox(dir => { dir.write({ @@ -547,6 +551,7 @@ describe('Node Package Importer', () => { }); }); })); + it('renderSync file', () => sandbox(dir => { dir.write({ @@ -562,6 +567,7 @@ describe('Node Package Importer', () => { expect(result).toEqualIgnoringWhitespace('a { b: c;}'); }); })); + it('renderSync data', () => sandbox(dir => { dir.write({ From 3bbb9189e68293438753d0e514b3211647abc849 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 16 Nov 2023 10:59:42 -0500 Subject: [PATCH 24/58] Check that no match in pkg importer continues to next importer --- js-api-spec/node-package-importer.node.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 5539721228..234a6b4cc9 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -407,7 +407,15 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect(() => compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], + importers: [ + nodePackageImporter, + { + canonicalize(url) { + expect(url).toStartWith('pkg:'); + }, + load: () => null, + }, + ], }) ).toThrowSassException({ includes: "Can't find stylesheet to import", From 4230a0ab9aeb8e660ebe78ba4e6bc015d4946f15 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 16 Nov 2023 11:24:04 -0500 Subject: [PATCH 25/58] Fix typo --- js-api-spec/node-package-importer.node.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 234a6b4cc9..8517d36c96 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -324,7 +324,7 @@ describe('Node Package Importer', () => { it('relative import in package', () => testPackageImporter({ input: '@use "pkg:foo";', - output: 'a {db: c;}', + output: 'a {b: c;}', files: { 'node_modules/foo/scss/styles.scss': '@use "mixins/banner";', 'node_modules/foo/scss/mixins/_banner.scss': 'a {b: c;}', From 137c8f9e55ff6d0ff94cef151f73e41c050e7615 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 17 Nov 2023 10:01:31 -0500 Subject: [PATCH 26/58] Handle invalid URLs --- .../node-package-importer.node.test.ts | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 8517d36c96..fb1ea2d442 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -591,4 +591,73 @@ describe('Node Package Importer', () => { }); })); }); + describe('rejects invalid URLs', () => { + it('with an absolute path', () => { + expect(() => + compileString('@use "pkg:/absolute";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'must not be an absolute path'}); + }); + + it('with a host', () => { + expect(() => + compileString('@use "pkg://host/library";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: 'must not have a host, port, username or password', + }); + }); + + it('with username and password', () => { + expect(() => + compileString('@use "pkg://user:password@library/path" as library;', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: 'must not have a host, port, username or password', + }); + }); + + it('with port', () => { + expect(() => + compileString('@use "pkg://host:8080/library";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: 'must not have a host, port, username or password', + }); + }); + + it('with an empty path', () => { + expect(() => + // Throws `default namespace "" is not a valid Sass identifier` without + // the `as` clause. + compileString('@use "pkg:" as pkg;', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'must not have an empty path'}); + }); + + it('with a query', () => { + expect(() => + // Throws `default namespace "" is not a valid Sass identifier` without + // the `as` clause. + compileString('@use "pkg:library?query";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'must not have a query or fragment'}); + }); + + it('with a fragment', () => { + expect(() => + // Throws `default namespace "" is not a valid Sass identifier` without + // the `as` clause. + compileString('@use "pkg:library#fragment";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'must not have a query or fragment'}); + }); + }); }); From 5a6382e313e75202a9c7e6e693a4d8aec4296c5b Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 17 Nov 2023 10:58:20 -0500 Subject: [PATCH 27/58] Adjust tests for Symbol and embedded host --- js-api-spec/node-package-importer.node.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index fb1ea2d442..7945007381 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -412,6 +412,7 @@ describe('Node Package Importer', () => { { canonicalize(url) { expect(url).toStartWith('pkg:'); + return null; }, load: () => null, }, @@ -429,7 +430,7 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({'foo/index.scss': 'a {from: dir}'}); - const foo = {_NodePackageImporterBrand: ''} as NodePackageImporter; + const foo = Symbol() as NodePackageImporter; expect(() => compileString('@use "pkg:foo";', { From 3d66845e2cba7dd20b25b3641b252577588d760a Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 17 Nov 2023 12:36:11 -0500 Subject: [PATCH 28/58] Update fake importer test --- js-api-spec/node-package-importer.node.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 7945007381..9fc89c48e2 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -430,11 +430,9 @@ describe('Node Package Importer', () => { sandbox(dir => { dir.write({'foo/index.scss': 'a {from: dir}'}); - const foo = Symbol() as NodePackageImporter; - expect(() => compileString('@use "pkg:foo";', { - importers: [foo], + importers: [Symbol() as NodePackageImporter], }) ).toThrow(); })); From 43086480394933ed954fbb6c85a2b6c6ca5cdd7e Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 22 Nov 2023 18:45:44 -0500 Subject: [PATCH 29/58] Test changes to entry point url --- .../node-package-importer.node.test.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 9fc89c48e2..c5aa6d545d 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -458,6 +458,28 @@ describe('Node Package Importer', () => { }); })); + it('compile with nested path', () => + sandbox(dir => { + dir.write({ + 'deeply/nested/node_modules/bah/index.scss': 'a {b: c}', + 'deeply/nested/node_modules/bah/package.json': JSON.stringify({}), + 'deeply/nested/_index.scss': '@use "pkg:bah";', + 'node_modules/bah/index.scss': 'from {root: notPath}', + 'node_modules/bah/package.json': JSON.stringify({}), + }); + dir.chdir(() => { + const result = compile('./deeply/nested/_index.scss', { + importers: [ + nodePackageImporter, + { + findFileUrl: file => dir.url(file), + }, + ], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); + })); + it('compileString', () => sandbox(dir => { dir.write({ @@ -472,6 +494,41 @@ describe('Node Package Importer', () => { }); })); + it('compileString with url', () => + sandbox(dir => { + dir.write({ + 'deeply/nested/node_modules/bah/index.scss': 'a {b: c}', + 'deeply/nested/node_modules/bah/package.json': JSON.stringify({}), + 'deeply/nested/_index.scss': '@use "pkg:bah";', + 'node_modules/bah/index.scss': 'from {root: notPath}', + 'node_modules/bah/package.json': JSON.stringify({}), + }); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + url: dir.url('deeply/nested/_index.scss'), + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); + })); + + it('compileString without url uses cwd', () => + sandbox(dir => { + dir.write({ + 'deeply/nested/node_modules/bah/index.scss': 'from {nested: path}', + 'deeply/nested/node_modules/bah/package.json': JSON.stringify({}), + 'deeply/nested/_index.scss': '@use "pkg:bah";', + 'node_modules/bah/index.scss': 'a {b: c}', + 'node_modules/bah/package.json': JSON.stringify({}), + }); + dir.chdir(() => { + const result = compileString('@use "pkg:bah";', { + importers: [nodePackageImporter], + }); + expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); + })); + it('compileAsync', () => sandbox(dir => { dir.write({ From 75b02ad448c00394de676182f4bf620f26b8121d Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 1 Dec 2023 09:12:34 -0500 Subject: [PATCH 30/58] Address review --- js-api-spec/legacy/importer.node.test.ts | 4 +--- js-api-spec/node-package-importer.node.test.ts | 17 +++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/js-api-spec/legacy/importer.node.test.ts b/js-api-spec/legacy/importer.node.test.ts index de2b68a480..081d36f536 100644 --- a/js-api-spec/legacy/importer.node.test.ts +++ b/js-api-spec/legacy/importer.node.test.ts @@ -63,9 +63,7 @@ describe('import precedence:', () => { sass .renderSync({ data: '@import "test"', - importer: () => { - return {contents: 'a {from: importer}'}; - }, + importer: () => ({contents: 'a {from: importer}'}), }) .css.toString() ).toEqualIgnoringWhitespace('a { from: importer; }') diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index c5aa6d545d..2716856451 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -17,6 +17,8 @@ import { import {sandbox} from './sandbox'; +import {spy} from './utils'; + const testPackageImporter = ({ input, output, @@ -403,6 +405,10 @@ describe('Node Package Importer', () => { })); it('fails if no match found', () => { + const canonicalize = spy((url: string) => { + expect(url).toStartWith('pkg:'); + return null; + }); sandbox(dir => { dir.chdir(() => { expect(() => @@ -410,10 +416,7 @@ describe('Node Package Importer', () => { importers: [ nodePackageImporter, { - canonicalize(url) { - expect(url).toStartWith('pkg:'); - return null; - }, + canonicalize, load: () => null, }, ], @@ -421,6 +424,7 @@ describe('Node Package Importer', () => { ).toThrowSassException({ includes: "Can't find stylesheet to import", }); + expect(canonicalize).toHaveBeenCalled(); }); }); }); @@ -647,6 +651,7 @@ describe('Node Package Importer', () => { }); })); }); + describe('rejects invalid URLs', () => { it('with an absolute path', () => { expect(() => @@ -698,8 +703,6 @@ describe('Node Package Importer', () => { it('with a query', () => { expect(() => - // Throws `default namespace "" is not a valid Sass identifier` without - // the `as` clause. compileString('@use "pkg:library?query";', { importers: [nodePackageImporter], }) @@ -708,8 +711,6 @@ describe('Node Package Importer', () => { it('with a fragment', () => { expect(() => - // Throws `default namespace "" is not a valid Sass identifier` without - // the `as` clause. compileString('@use "pkg:library#fragment";', { importers: [nodePackageImporter], }) From 5b3819508fd63ed12a9259570873697923d8f0e8 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 1 Dec 2023 11:19:59 -0500 Subject: [PATCH 31/58] Test for non-sass file result --- .../node-package-importer.node.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 2716856451..af6cbcf708 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -144,6 +144,26 @@ describe('Node Package Importer', () => { }); }); })); + it('throws if resolved path does not have a valid extension', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.txt': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + './index.scss': {sass: './src/sass/_styles.txt'}, + }, + }), + }); + dir.chdir(() => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: "_styles.txt', which is not a '.scss'", + }); + }); + })); it('resolves string export', () => testPackageImporter({ From 247a136712c7b730209d432e043bdb0deb0a0085 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 1 Dec 2023 15:36:37 -0500 Subject: [PATCH 32/58] Test throw if invalid package.json --- js-api-spec/node-package-importer.node.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index af6cbcf708..e2ec730557 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -144,6 +144,7 @@ describe('Node Package Importer', () => { }); }); })); + it('throws if resolved path does not have a valid extension', () => sandbox(dir => { dir.write({ @@ -249,6 +250,22 @@ describe('Node Package Importer', () => { }); }); + it('throws if package.json is not json', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/package.json': 'invalid json', + }); + dir.chdir(() => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({ + includes: "'package.json' in 'pkg:foo' cannot be parsed", + }); + }); + })); + describe('without subpath', () => { it('sass key in package.json', () => testPackageImporter({ From 0cd68eb8c3b4d3ba7166163bc14f6964bc7e4fc8 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Mon, 4 Dec 2023 11:37:27 -0600 Subject: [PATCH 33/58] Add test for exports with and without . --- .../node-package-importer.node.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index e2ec730557..a6536b13a4 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -478,6 +478,28 @@ describe('Node Package Importer', () => { ).toThrow(); })); + it('fails with invalid package.json exports', () => + sandbox(dir => { + dir.write({ + 'node_modules/foo/src/sass/_styles.scss': 'a {b: c}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + '.': { + sass: './src/sass/_styles.scss', + }, + sass: './src/sass/_styles.scss', + }, + }), + }); + dir.chdir(() => { + expect(() => + compileString('@use "pkg:foo";', { + importers: [nodePackageImporter], + }) + ).toThrowSassException({includes: 'Invalid Package Configuration'}); + }); + })); + describe('compilation methods', () => { it('compile', () => sandbox(dir => { From 31b757a2ae336397f61626b5a2bd7dbf45c4642a Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Tue, 19 Dec 2023 15:19:37 -0500 Subject: [PATCH 34/58] Move to Node Package Importer Class, add test for explicit entry point --- .../node-package-importer.node.test.ts | 73 ++++++++++++------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index a6536b13a4..8c1dfcaa9c 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -9,7 +9,6 @@ import { compileStringAsync, render, renderSync, - nodePackageImporter, NodePackageImporter, LegacyException, LegacyResult, @@ -23,16 +22,18 @@ const testPackageImporter = ({ input, output, files, + entryPoint, }: { input: string; output: string; files: {[path: string]: string}; + entryPoint?: string; }) => sandbox(dir => { dir.write(files); dir.chdir(() => { const result = compileString(input, { - importers: [nodePackageImporter], + importers: [new NodePackageImporter(entryPoint)], }); expect(result.css).toEqualIgnoringWhitespace(output); }); @@ -137,7 +138,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: 'multiple potential resolutions', @@ -158,7 +159,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: "_styles.txt', which is not a '.scss'", @@ -240,7 +241,7 @@ describe('Node Package Importer', () => { expect( () => compileString('@use "pkg:foo/styles";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }).css ).toThrowSassException({ includes: 'multiple potential resolutions', @@ -258,7 +259,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: "'package.json' in 'pkg:foo' cannot be parsed", @@ -338,7 +339,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { const result = compileString('@use "vendor";', { importers: [ - nodePackageImporter, + new NodePackageImporter(), { findFileUrl: file => dir.url(file), }, @@ -388,6 +389,22 @@ describe('Node Package Importer', () => { }, })); + it('resolves most proximate node_module to specified entry point', () => + testPackageImporter({ + input: '@use "pkg:bah";', + output: 'a {from: submodule;}', + files: { + 'subdir/node_modules/bah/index.scss': '@use "pkg:bar";', + 'subdir/node_modules/bah/package.json': JSON.stringify({}), + 'node_modules/bar/index.scss': 'e {from: root}', + 'node_modules/bar/package.json': JSON.stringify({}), + 'subdir/node_modules/bah/node_modules/bar/index.scss': + 'a {from: submodule;}', + 'subdir/node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), + }, + entryPoint: './subdir/index.js', + })); + it('resolves sub node_module', () => testPackageImporter({ input: '@use "pkg:bah";', @@ -409,7 +426,7 @@ describe('Node Package Importer', () => { dir.chdir( () => { const result = compileString('@use "pkg:bar";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, @@ -451,7 +468,7 @@ describe('Node Package Importer', () => { expect(() => compileString('@use "pkg:bah";', { importers: [ - nodePackageImporter, + new NodePackageImporter(), { canonicalize, load: () => null, @@ -494,7 +511,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({includes: 'Invalid Package Configuration'}); }); @@ -511,7 +528,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { const result = compile('./_index.scss', { importers: [ - nodePackageImporter, + new NodePackageImporter(), { findFileUrl: file => dir.url(file), }, @@ -533,7 +550,7 @@ describe('Node Package Importer', () => { dir.chdir(() => { const result = compile('./deeply/nested/_index.scss', { importers: [ - nodePackageImporter, + new NodePackageImporter(), { findFileUrl: file => dir.url(file), }, @@ -551,7 +568,7 @@ describe('Node Package Importer', () => { }); dir.chdir(() => { const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }); @@ -568,7 +585,7 @@ describe('Node Package Importer', () => { }); dir.chdir(() => { const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], url: dir.url('deeply/nested/_index.scss'), }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -586,7 +603,7 @@ describe('Node Package Importer', () => { }); dir.chdir(() => { const result = compileString('@use "pkg:bah";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }); @@ -602,7 +619,7 @@ describe('Node Package Importer', () => { return dir.chdir(async () => { const result = await compileAsync('./_index.scss', { importers: [ - nodePackageImporter, + new NodePackageImporter(), { findFileUrl: file => dir.url(file), }, @@ -621,7 +638,7 @@ describe('Node Package Importer', () => { }); return dir.chdir(async () => { const result = await compileStringAsync('@use "pkg:bah";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }); expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); return result; @@ -639,7 +656,7 @@ describe('Node Package Importer', () => { render( { data: '@use "pkg:bah"', - pkgImporter: 'node', + pkgImporter: {type: 'node'}, }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); @@ -665,7 +682,7 @@ describe('Node Package Importer', () => { render( { file: 'index.scss', - pkgImporter: 'node', + pkgImporter: {type: 'node'}, }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); @@ -689,7 +706,7 @@ describe('Node Package Importer', () => { return dir.chdir(() => { const result = renderSync({ file: 'index.scss', - pkgImporter: 'node', + pkgImporter: {type: 'node'}, }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); }); @@ -704,7 +721,7 @@ describe('Node Package Importer', () => { return dir.chdir(() => { const result = renderSync({ data: '@use "pkg:bah"', - pkgImporter: 'node', + pkgImporter: {type: 'node'}, }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); }); @@ -715,7 +732,7 @@ describe('Node Package Importer', () => { it('with an absolute path', () => { expect(() => compileString('@use "pkg:/absolute";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({includes: 'must not be an absolute path'}); }); @@ -723,7 +740,7 @@ describe('Node Package Importer', () => { it('with a host', () => { expect(() => compileString('@use "pkg://host/library";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: 'must not have a host, port, username or password', @@ -733,7 +750,7 @@ describe('Node Package Importer', () => { it('with username and password', () => { expect(() => compileString('@use "pkg://user:password@library/path" as library;', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: 'must not have a host, port, username or password', @@ -743,7 +760,7 @@ describe('Node Package Importer', () => { it('with port', () => { expect(() => compileString('@use "pkg://host:8080/library";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({ includes: 'must not have a host, port, username or password', @@ -755,7 +772,7 @@ describe('Node Package Importer', () => { // Throws `default namespace "" is not a valid Sass identifier` without // the `as` clause. compileString('@use "pkg:" as pkg;', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({includes: 'must not have an empty path'}); }); @@ -763,7 +780,7 @@ describe('Node Package Importer', () => { it('with a query', () => { expect(() => compileString('@use "pkg:library?query";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({includes: 'must not have a query or fragment'}); }); @@ -771,7 +788,7 @@ describe('Node Package Importer', () => { it('with a fragment', () => { expect(() => compileString('@use "pkg:library#fragment";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrowSassException({includes: 'must not have a query or fragment'}); }); From 1a292f5aa3c6d02ed14a9aa682a4dbd7ceb9b9be Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Tue, 19 Dec 2023 15:22:42 -0500 Subject: [PATCH 35/58] lint --- js-api-spec/node-package-importer.node.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 8c1dfcaa9c..8eb0914fa2 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -400,7 +400,8 @@ describe('Node Package Importer', () => { 'node_modules/bar/package.json': JSON.stringify({}), 'subdir/node_modules/bah/node_modules/bar/index.scss': 'a {from: submodule;}', - 'subdir/node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), + 'subdir/node_modules/bah/node_modules/bar/package.json': + JSON.stringify({}), }, entryPoint: './subdir/index.js', })); From a31051f6183fe0f7845f214023f3c7dd8fc1622c Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Tue, 19 Dec 2023 16:18:45 -0500 Subject: [PATCH 36/58] Fix importer test --- js-api-spec/importer.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js-api-spec/importer.test.ts b/js-api-spec/importer.test.ts index 8c9839e820..1ca2037ed8 100644 --- a/js-api-spec/importer.test.ts +++ b/js-api-spec/importer.test.ts @@ -7,7 +7,7 @@ import { compileStringAsync, CanonicalizeContext, Importer, - nodePackageImporter, + NodePackageImporter, } from 'sass'; import {sassImpl, runOnlyForImpl, URL} from './utils'; @@ -768,7 +768,7 @@ runOnlyForImpl('browser', () => { it('node package loader throws error in browser', () => { expect(() => compileString('@use "pkg:foo";', { - importers: [nodePackageImporter], + importers: [new NodePackageImporter()], }) ).toThrow(); }); From ad97abb8ea938b6901c5dadde2c8088867759311 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Wed, 20 Dec 2023 11:42:47 -0500 Subject: [PATCH 37/58] Add tests for package name errors --- .../node-package-importer.node.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 8eb0914fa2..f09b238ce1 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -793,5 +793,35 @@ describe('Node Package Importer', () => { }) ).toThrowSassException({includes: 'must not have a query or fragment'}); }); + + describe('with package name', () => { + it('starting with a .', () => { + // Throws `default namespace "" is not a valid Sass identifier` without + // the `as` clause. + expect(() => + compileString('@use "pkg:.library" as library;', { + importers: [new NodePackageImporter()], + }) + ).toThrowSassException({ + includes: ".library must not start with a '.'", + }); + }); + + it('with scope but no segment', () => { + expect(() => + compileString('@use "pkg:@library" as library;', { + importers: [new NodePackageImporter()], + }) + ).toThrowSassException({includes: 'must have a second segment.'}); + }); + + it('with %', () => { + expect(() => + compileString('@use "pkg:library%" as library;', { + importers: [new NodePackageImporter()], + }) + ).toThrowSassException({includes: "must not contain a '%'"}); + }); + }); }); }); From 654b4517662bf7be7fec171e6bc4feebff34516a Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Wed, 20 Dec 2023 12:29:43 -0500 Subject: [PATCH 38/58] Temporarily log full stack trace on error --- js-api-spec/node-package-importer.node.test.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index f09b238ce1..ec947edcc0 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -32,10 +32,15 @@ const testPackageImporter = ({ sandbox(dir => { dir.write(files); dir.chdir(() => { - const result = compileString(input, { - importers: [new NodePackageImporter(entryPoint)], - }); - expect(result.css).toEqualIgnoringWhitespace(output); + try { + const result = compileString(input, { + importers: [new NodePackageImporter(entryPoint)], + }); + expect(result.css).toEqualIgnoringWhitespace(output); + } catch (error) { + console.error(error); + throw error; + } }); }); From 3fdd16331940de5764241d34bb8704ee3dde05b7 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Wed, 20 Dec 2023 15:14:17 -0500 Subject: [PATCH 39/58] add comment --- js-api-spec/node-package-importer.node.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index ec947edcc0..5df6abc8d7 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -38,6 +38,7 @@ const testPackageImporter = ({ }); expect(result.css).toEqualIgnoringWhitespace(output); } catch (error) { + // Log error to include full stack trace console.error(error); throw error; } From bee93ca9b8f49b549ffacdfd212c6161f7dcf77e Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 5 Jan 2024 11:26:46 -0500 Subject: [PATCH 40/58] Update error messages --- js-api-spec/node-package-importer.node.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 5df6abc8d7..7ff7fa6332 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -268,7 +268,7 @@ describe('Node Package Importer', () => { importers: [new NodePackageImporter()], }) ).toThrowSassException({ - includes: "'package.json' in 'pkg:foo' cannot be parsed", + includes: 'Failed to parse', }); }); })); @@ -520,7 +520,9 @@ describe('Node Package Importer', () => { compileString('@use "pkg:foo";', { importers: [new NodePackageImporter()], }) - ).toThrowSassException({includes: 'Invalid Package Configuration'}); + ).toThrowSassException({ + includes: 'can not have both conditions and paths', + }); }); })); From 2b6a3c5cefd7c9eea1609f4c2f4bf66d49db865b Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Fri, 5 Jan 2024 11:51:27 -0500 Subject: [PATCH 41/58] Update legacy tests for new pkgImporter syntax --- js-api-spec/node-package-importer.node.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 7ff7fa6332..eabf48266d 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -347,7 +347,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: file => dir.url(file), + findFileUrl: (file: string) => dir.url(file), }, ], }); @@ -539,7 +539,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: file => dir.url(file), + findFileUrl: (file: string) => dir.url(file), }, ], }); @@ -561,7 +561,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: file => dir.url(file), + findFileUrl: (file: string) => dir.url(file), }, ], }); @@ -630,7 +630,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: file => dir.url(file), + findFileUrl: (file: string) => dir.url(file), }, ], }); @@ -665,7 +665,7 @@ describe('Node Package Importer', () => { render( { data: '@use "pkg:bah"', - pkgImporter: {type: 'node'}, + pkgImporter: new NodePackageImporter(), }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); @@ -691,7 +691,7 @@ describe('Node Package Importer', () => { render( { file: 'index.scss', - pkgImporter: {type: 'node'}, + pkgImporter: new NodePackageImporter(), }, (err?: LegacyException, result?: LegacyResult) => { expect(err).toBeFalsy(); @@ -715,7 +715,7 @@ describe('Node Package Importer', () => { return dir.chdir(() => { const result = renderSync({ file: 'index.scss', - pkgImporter: {type: 'node'}, + pkgImporter: new NodePackageImporter(), }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); }); @@ -730,7 +730,7 @@ describe('Node Package Importer', () => { return dir.chdir(() => { const result = renderSync({ data: '@use "pkg:bah"', - pkgImporter: {type: 'node'}, + pkgImporter: new NodePackageImporter(), }).css.toString(); expect(result).toEqualIgnoringWhitespace('a { b: c;}'); }); From a6cdbdf6b85d745dd26b28e6d897452211754b26 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Fri, 5 Jan 2024 11:56:42 -0500 Subject: [PATCH 42/58] whitespace --- js-api-spec/node-package-importer.node.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index eabf48266d..6430a6c9d6 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -15,7 +15,6 @@ import { } from 'sass'; import {sandbox} from './sandbox'; - import {spy} from './utils'; const testPackageImporter = ({ @@ -311,6 +310,7 @@ describe('Node Package Importer', () => { })); } ); + ['index.sass', '_index.sass'].forEach(fileName => { it(`loads from ${fileName}`, () => testPackageImporter({ From 46b38be5b60ff5a85c2749d04830c821d0419521 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Fri, 5 Jan 2024 15:52:48 -0500 Subject: [PATCH 43/58] update copyright year --- js-api-spec/node-package-importer.node.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 6430a6c9d6..ffc27bf089 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -1,4 +1,4 @@ -// Copyright 2023 Google Inc. Use of this source code is governed by an +// Copyright 2024 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. From 5f7631433ec84d3e03fa61e94c2a0cfd6e442c70 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Sun, 7 Jan 2024 21:31:38 -0500 Subject: [PATCH 44/58] Update package name tests --- .../node-package-importer.node.test.ts | 74 ++++++++++++------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index ffc27bf089..f9ec40922f 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -801,35 +801,59 @@ describe('Node Package Importer', () => { }) ).toThrowSassException({includes: 'must not have a query or fragment'}); }); + }); - describe('with package name', () => { - it('starting with a .', () => { - // Throws `default namespace "" is not a valid Sass identifier` without - // the `as` clause. - expect(() => - compileString('@use "pkg:.library" as library;', { - importers: [new NodePackageImporter()], - }) - ).toThrowSassException({ - includes: ".library must not start with a '.'", - }); + describe('package name rules', () => { + it('no match starting with a .', () => { + const canonicalize = spy((url: string) => { + expect(url).toStartWith('pkg:.'); + return null; }); - - it('with scope but no segment', () => { - expect(() => - compileString('@use "pkg:@library" as library;', { - importers: [new NodePackageImporter()], - }) - ).toThrowSassException({includes: 'must have a second segment.'}); + // Throws `default namespace "" is not a valid Sass identifier` without + // the `as` clause. + expect(() => + compileString('@use "pkg:.library" as library;', { + importers: [ + new NodePackageImporter(), + {canonicalize, load: () => null}, + ], + }) + ).toThrowSassException({ + includes: "Can't find stylesheet to import", }); + expect(canonicalize).toHaveBeenCalled(); + }); - it('with %', () => { - expect(() => - compileString('@use "pkg:library%" as library;', { - importers: [new NodePackageImporter()], - }) - ).toThrowSassException({includes: "must not contain a '%'"}); - }); + it('throws with scope but no segment', () => { + expect(() => + compileString('@use "pkg:@library" as library;', { + importers: [new NodePackageImporter()], + }) + ).toThrowSassException({includes: 'must have a second segment.'}); }); + + it('throws with escaped %', () => { + expect(() => + compileString('@use "pkg:library%" as library;', { + importers: [new NodePackageImporter()], + }) + ).toThrowSassException({includes: "must not contain a '%'"}); + }); + + it('passes with parsed %', () => + testPackageImporter({ + input: '@use "pkg:%66oo";', + output: 'a {from: sassCondition;}', + files: { + 'node_modules/foo/src/sass/_sass.scss': 'a {from: sassCondition}', + 'node_modules/foo/package.json': JSON.stringify({ + exports: { + '.': { + sass: './src/sass/_sass.scss', + }, + }, + }), + }, + })); }); }); From 27ee25b515a89913919299126cd3d2ac6fc10b76 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Mon, 8 Jan 2024 14:32:52 -0500 Subject: [PATCH 45/58] remove unnecessary type defs --- js-api-spec/node-package-importer.node.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index f9ec40922f..f2340ba9e2 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -347,7 +347,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: (file: string) => dir.url(file), + findFileUrl: file => dir.url(file), }, ], }); @@ -539,7 +539,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: (file: string) => dir.url(file), + findFileUrl: file => dir.url(file), }, ], }); @@ -561,7 +561,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: (file: string) => dir.url(file), + findFileUrl: file => dir.url(file), }, ], }); @@ -630,7 +630,7 @@ describe('Node Package Importer', () => { importers: [ new NodePackageImporter(), { - findFileUrl: (file: string) => dir.url(file), + findFileUrl: file => dir.url(file), }, ], }); From 3f435d77a884a5f1f94e9ce4bd7f25b918245744 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 18 Jan 2024 11:09:58 -0500 Subject: [PATCH 46/58] Make chdir async in sandbox --- js-api-spec/sandbox.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/js-api-spec/sandbox.ts b/js-api-spec/sandbox.ts index 0a542e5a55..6ac8be899a 100644 --- a/js-api-spec/sandbox.ts +++ b/js-api-spec/sandbox.ts @@ -49,7 +49,10 @@ export async function sandbox( fs.writeFileSync(fullPath, contents); } }, - chdir: (callback: () => unknown, options?: {entryPoint: string}) => { + chdir: async ( + callback: () => unknown, + options?: {entryPoint: string} + ) => { const oldPath = process.cwd(); process.chdir(testDir); const oldEntryPoint = require.main?.filename; @@ -58,7 +61,7 @@ export async function sandbox( require.main!.filename = `${testDir}/${filename}`; } try { - return callback(); + return await callback(); } finally { process.chdir(oldPath); if (oldEntryPoint) require.main!.filename = oldEntryPoint; From a01db2a10a8b8a52cf9d625b047a3fc41f54a500 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 18 Jan 2024 23:16:13 -0500 Subject: [PATCH 47/58] Update expected errors --- js-api-spec/node-package-importer.node.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index f2340ba9e2..e925666506 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -743,7 +743,7 @@ describe('Node Package Importer', () => { compileString('@use "pkg:/absolute";', { importers: [new NodePackageImporter()], }) - ).toThrowSassException({includes: 'must not be an absolute path'}); + ).toThrowSassException({includes: 'must not begin with /'}); }); it('with a host', () => { From d20e8b4a0e38fd591ad8a692c8fab959b795d5b3 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 19 Jan 2024 09:42:09 -0500 Subject: [PATCH 48/58] No error with invalid node package names --- .../node-package-importer.node.test.ts | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index e925666506..77e05ed93f 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -824,20 +824,40 @@ describe('Node Package Importer', () => { expect(canonicalize).toHaveBeenCalled(); }); - it('throws with scope but no segment', () => { + it('no match with scope but no segment', () => { + const canonicalize = spy((url: string) => { + expect(url).toStartWith('pkg:@library'); + return null; + }); expect(() => compileString('@use "pkg:@library" as library;', { - importers: [new NodePackageImporter()], + importers: [ + new NodePackageImporter(), + {canonicalize, load: () => null}, + ], }) - ).toThrowSassException({includes: 'must have a second segment.'}); + ).toThrowSassException({ + includes: "Can't find stylesheet to import", + }); + expect(canonicalize).toHaveBeenCalled(); }); - it('throws with escaped %', () => { + it('no match with escaped %', () => { + const canonicalize = spy((url: string) => { + expect(url).toStartWith('pkg:library%'); + return null; + }); expect(() => compileString('@use "pkg:library%" as library;', { - importers: [new NodePackageImporter()], + importers: [ + new NodePackageImporter(), + {canonicalize, load: () => null}, + ], }) - ).toThrowSassException({includes: "must not contain a '%'"}); + ).toThrowSassException({ + includes: "Can't find stylesheet to import", + }); + expect(canonicalize).toHaveBeenCalled(); }); it('passes with parsed %', () => From b18ff2ca5bb3dfbdc33928729ba3abda226cd553 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Wed, 24 Jan 2024 18:41:20 -0500 Subject: [PATCH 49/58] Address review --- js-api-spec/importer.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/js-api-spec/importer.test.ts b/js-api-spec/importer.test.ts index 1ca2037ed8..f2bd971542 100644 --- a/js-api-spec/importer.test.ts +++ b/js-api-spec/importer.test.ts @@ -766,11 +766,7 @@ it('throws an ArgumentError when the result sourceMapUrl is missing a scheme', ( runOnlyForImpl('browser', () => { it('node package loader throws error in browser', () => { - expect(() => - compileString('@use "pkg:foo";', { - importers: [new NodePackageImporter()], - }) - ).toThrow(); + expect(() => new NodePackageImporter()).toThrow(); }); }); From 7a5e0ce1237f1fafdcc0de1aaa732a8216519659 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 12:01:23 -0500 Subject: [PATCH 50/58] Update entry point to directory --- .../node-package-importer.node.test.ts | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 77e05ed93f..7bf5e83b98 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -30,7 +30,7 @@ const testPackageImporter = ({ }) => sandbox(dir => { dir.write(files); - dir.chdir(() => { + return dir.chdir(() => { try { const result = compileString(input, { importers: [new NodePackageImporter(entryPoint)], @@ -402,14 +402,14 @@ describe('Node Package Importer', () => { files: { 'subdir/node_modules/bah/index.scss': '@use "pkg:bar";', 'subdir/node_modules/bah/package.json': JSON.stringify({}), - 'node_modules/bar/index.scss': 'e {from: root}', - 'node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bah/index.scss': 'e {from: root}', + 'node_modules/bah/package.json': JSON.stringify({}), 'subdir/node_modules/bah/node_modules/bar/index.scss': 'a {from: submodule;}', 'subdir/node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), }, - entryPoint: './subdir/index.js', + entryPoint: './subdir', })); it('resolves sub node_module', () => @@ -437,7 +437,24 @@ describe('Node Package Importer', () => { }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - {entryPoint: 'deeply/nested/file/index.js'} + {entryPoint: 'deeply/nested/file/'} + ); + })); + + it('resolves with absolute entry point directory', () => + sandbox(dir => { + dir.write({ + 'node_modules/bar/index.scss': 'a {b: c}', + 'node_modules/bar/package.json': JSON.stringify({}), + }); + dir.chdir( + () => { + const result = compileString('@use "pkg:bar";', { + importers: [new NodePackageImporter()], + }); + return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }, + {entryPoint: dir.url().pathname} ); })); From ce63d685a266eb11980d090d0f06676dd8b61b36 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 12:54:02 -0500 Subject: [PATCH 51/58] Make pkg:bar unique for troubleshooting --- .../node-package-importer.node.test.ts | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 7bf5e83b98..05b417e8c0 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -360,10 +360,10 @@ describe('Node Package Importer', () => { input: '@use "pkg:bah";', output: 'a {b: c;}', files: { - 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/index.scss': '@use "pkg:bar-secondary";', 'node_modules/bah/package.json': JSON.stringify({}), - 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bar-secondary/index.scss': 'a {b: c}', + 'node_modules/bar-secondary/package.json': JSON.stringify({}), }, })); @@ -385,13 +385,13 @@ describe('Node Package Importer', () => { input: '@use "pkg:bah";', output: 'a {from: submodule;}', files: { - 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/index.scss': '@use "pkg:bar-proximate";', 'node_modules/bah/package.json': JSON.stringify({}), - 'node_modules/bar/index.scss': 'e {from: root}', - 'node_modules/bar/package.json': JSON.stringify({}), - 'node_modules/bah/node_modules/bar/index.scss': + 'node_modules/bar-proximate/index.scss': 'e {from: root}', + 'node_modules/bar-proximate/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar-proximate/index.scss': 'a {from: submodule;}', - 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar-proximate/package.json': JSON.stringify({}), }, })); @@ -400,13 +400,13 @@ describe('Node Package Importer', () => { input: '@use "pkg:bah";', output: 'a {from: submodule;}', files: { - 'subdir/node_modules/bah/index.scss': '@use "pkg:bar";', + 'subdir/node_modules/bah/index.scss': '@use "pkg:bar-entry";', 'subdir/node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bah/index.scss': 'e {from: root}', 'node_modules/bah/package.json': JSON.stringify({}), - 'subdir/node_modules/bah/node_modules/bar/index.scss': + 'subdir/node_modules/bah/node_modules/bar-entry/index.scss': 'a {from: submodule;}', - 'subdir/node_modules/bah/node_modules/bar/package.json': + 'subdir/node_modules/bah/node_modules/bar-entry/package.json': JSON.stringify({}), }, entryPoint: './subdir', @@ -417,22 +417,22 @@ describe('Node Package Importer', () => { input: '@use "pkg:bah";', output: 'a {b: c;}', files: { - 'node_modules/bah/index.scss': '@use "pkg:bar";', + 'node_modules/bah/index.scss': '@use "pkg:bar-sub";', 'node_modules/bah/package.json': JSON.stringify({}), - 'node_modules/bah/node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar-sub/index.scss': 'a {b: c}', + 'node_modules/bah/node_modules/bar-sub/package.json': JSON.stringify({}), }, })); it('resolves node_module above cwd', () => sandbox(dir => { dir.write({ - 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bar-above/index.scss': 'a {b: c}', + 'node_modules/bar-above/package.json': JSON.stringify({}), }); dir.chdir( () => { - const result = compileString('@use "pkg:bar";', { + const result = compileString('@use "pkg:bar-above";', { importers: [new NodePackageImporter()], }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); @@ -444,12 +444,12 @@ describe('Node Package Importer', () => { it('resolves with absolute entry point directory', () => sandbox(dir => { dir.write({ - 'node_modules/bar/index.scss': 'a {b: c}', - 'node_modules/bar/package.json': JSON.stringify({}), + 'node_modules/bar-abs/index.scss': 'a {b: c}', + 'node_modules/bar-abs/package.json': JSON.stringify({}), }); dir.chdir( () => { - const result = compileString('@use "pkg:bar";', { + const result = compileString('@use "pkg:bar-abs";', { importers: [new NodePackageImporter()], }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); From 029ce09bed0233ac23868aae250f1365c8e28103 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 12:57:03 -0500 Subject: [PATCH 52/58] Lint --- js-api-spec/node-package-importer.node.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 05b417e8c0..2fd6fae1e2 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -391,7 +391,8 @@ describe('Node Package Importer', () => { 'node_modules/bar-proximate/package.json': JSON.stringify({}), 'node_modules/bah/node_modules/bar-proximate/index.scss': 'a {from: submodule;}', - 'node_modules/bah/node_modules/bar-proximate/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar-proximate/package.json': + JSON.stringify({}), }, })); @@ -420,7 +421,9 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': '@use "pkg:bar-sub";', 'node_modules/bah/package.json': JSON.stringify({}), 'node_modules/bah/node_modules/bar-sub/index.scss': 'a {b: c}', - 'node_modules/bah/node_modules/bar-sub/package.json': JSON.stringify({}), + 'node_modules/bah/node_modules/bar-sub/package.json': JSON.stringify( + {} + ), }, })); From 628add25166eca8a6ef2b6c6c48a385d35fe82d5 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 13:24:25 -0500 Subject: [PATCH 53/58] Log test error --- js-api-spec/node-package-importer.node.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 2fd6fae1e2..358e4bcd2d 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -450,14 +450,16 @@ describe('Node Package Importer', () => { 'node_modules/bar-abs/index.scss': 'a {b: c}', 'node_modules/bar-abs/package.json': JSON.stringify({}), }); - dir.chdir( + const entryPoint = dir.url().pathname; + console.log(`bar-abs entryPoint: ${entryPoint}`); + return dir.chdir( () => { const result = compileString('@use "pkg:bar-abs";', { importers: [new NodePackageImporter()], }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - {entryPoint: dir.url().pathname} + {entryPoint} ); })); From c841cce4fe701a5da9f707e1f2686f34ec93d709 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 13:39:28 -0500 Subject: [PATCH 54/58] Return chdir to ensure errors don't leak, use file url toString for absolute path --- .../node-package-importer.node.test.ts | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 358e4bcd2d..d65141fc96 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -140,7 +140,7 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir(() => { + return dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { importers: [new NodePackageImporter()], @@ -161,7 +161,7 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir(() => { + return dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { importers: [new NodePackageImporter()], @@ -242,7 +242,7 @@ describe('Node Package Importer', () => { exports: {'./*.scss': './src/sass/*.scss'}, }), }); - dir.chdir(() => { + return dir.chdir(() => { expect( () => compileString('@use "pkg:foo/styles";', { @@ -261,7 +261,7 @@ describe('Node Package Importer', () => { dir.write({ 'node_modules/foo/package.json': 'invalid json', }); - dir.chdir(() => { + return dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { importers: [new NodePackageImporter()], @@ -342,7 +342,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), '_vendor.scss': '@use "pkg:bah";', }); - dir.chdir(() => { + return dir.chdir(() => { const result = compileString('@use "vendor";', { importers: [ new NodePackageImporter(), @@ -433,7 +433,7 @@ describe('Node Package Importer', () => { 'node_modules/bar-above/index.scss': 'a {b: c}', 'node_modules/bar-above/package.json': JSON.stringify({}), }); - dir.chdir( + return dir.chdir( () => { const result = compileString('@use "pkg:bar-above";', { importers: [new NodePackageImporter()], @@ -444,14 +444,12 @@ describe('Node Package Importer', () => { ); })); - it('resolves with absolute entry point directory', () => + fit('resolves with absolute entry point directory', () => sandbox(dir => { dir.write({ 'node_modules/bar-abs/index.scss': 'a {b: c}', 'node_modules/bar-abs/package.json': JSON.stringify({}), }); - const entryPoint = dir.url().pathname; - console.log(`bar-abs entryPoint: ${entryPoint}`); return dir.chdir( () => { const result = compileString('@use "pkg:bar-abs";', { @@ -459,7 +457,7 @@ describe('Node Package Importer', () => { }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - {entryPoint} + {entryPoint: dir.url().toString()} ); })); @@ -493,7 +491,7 @@ describe('Node Package Importer', () => { return null; }); sandbox(dir => { - dir.chdir(() => { + return dir.chdir(() => { expect(() => compileString('@use "pkg:bah";', { importers: [ @@ -537,7 +535,7 @@ describe('Node Package Importer', () => { }, }), }); - dir.chdir(() => { + return dir.chdir(() => { expect(() => compileString('@use "pkg:foo";', { importers: [new NodePackageImporter()], @@ -556,7 +554,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/package.json': JSON.stringify({}), '_index.scss': '@use "pkg:bah";', }); - dir.chdir(() => { + return dir.chdir(() => { const result = compile('./_index.scss', { importers: [ new NodePackageImporter(), @@ -578,7 +576,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'from {root: notPath}', 'node_modules/bah/package.json': JSON.stringify({}), }); - dir.chdir(() => { + return dir.chdir(() => { const result = compile('./deeply/nested/_index.scss', { importers: [ new NodePackageImporter(), @@ -597,7 +595,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - dir.chdir(() => { + return dir.chdir(() => { const result = compileString('@use "pkg:bah";', { importers: [new NodePackageImporter()], }); @@ -614,7 +612,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'from {root: notPath}', 'node_modules/bah/package.json': JSON.stringify({}), }); - dir.chdir(() => { + return dir.chdir(() => { const result = compileString('@use "pkg:bah";', { importers: [new NodePackageImporter()], url: dir.url('deeply/nested/_index.scss'), @@ -632,7 +630,7 @@ describe('Node Package Importer', () => { 'node_modules/bah/index.scss': 'a {b: c}', 'node_modules/bah/package.json': JSON.stringify({}), }); - dir.chdir(() => { + return dir.chdir(() => { const result = compileString('@use "pkg:bah";', { importers: [new NodePackageImporter()], }); From 45561f44134f259898521383189caf6c13dfd0b4 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 13:53:56 -0500 Subject: [PATCH 55/58] Remove skips, relog --- js-api-spec/node-package-importer.node.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index d65141fc96..51ecd05cb3 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -444,12 +444,14 @@ describe('Node Package Importer', () => { ); })); - fit('resolves with absolute entry point directory', () => + it('resolves with absolute entry point directory', () => sandbox(dir => { dir.write({ 'node_modules/bar-abs/index.scss': 'a {b: c}', 'node_modules/bar-abs/package.json': JSON.stringify({}), }); + const entryPoint = dir.url().toString(); + console.log(`pkg:bar-abs entrypoint- ${entryPoint}`); return dir.chdir( () => { const result = compileString('@use "pkg:bar-abs";', { @@ -457,7 +459,7 @@ describe('Node Package Importer', () => { }); return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); }, - {entryPoint: dir.url().toString()} + {entryPoint} ); })); From adf2f4aab614ae2e43d9c7d00847f65dd670b9ca Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 14:11:31 -0500 Subject: [PATCH 56/58] Try pathname --- js-api-spec/node-package-importer.node.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 51ecd05cb3..151d73daea 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -450,7 +450,7 @@ describe('Node Package Importer', () => { 'node_modules/bar-abs/index.scss': 'a {b: c}', 'node_modules/bar-abs/package.json': JSON.stringify({}), }); - const entryPoint = dir.url().toString(); + const entryPoint = dir.url().pathname; console.log(`pkg:bar-abs entrypoint- ${entryPoint}`); return dir.chdir( () => { From 2e5b770eb08b825766eb00d2164c0c5163c093d2 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 14:24:00 -0500 Subject: [PATCH 57/58] Use fileURLToPath --- js-api-spec/node-package-importer.node.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index 151d73daea..ac99bf15c0 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -17,6 +17,8 @@ import { import {sandbox} from './sandbox'; import {spy} from './utils'; +import {fileURLToPath} from 'url'; + const testPackageImporter = ({ input, output, @@ -450,7 +452,7 @@ describe('Node Package Importer', () => { 'node_modules/bar-abs/index.scss': 'a {b: c}', 'node_modules/bar-abs/package.json': JSON.stringify({}), }); - const entryPoint = dir.url().pathname; + const entryPoint = fileURLToPath(dir.url()); console.log(`pkg:bar-abs entrypoint- ${entryPoint}`); return dir.chdir( () => { From 3263bc325c52d477a518f013d43e85398a12b8c2 Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Thu, 1 Feb 2024 14:53:44 -0500 Subject: [PATCH 58/58] Pass entrypoint correctly --- js-api-spec/node-package-importer.node.test.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/js-api-spec/node-package-importer.node.test.ts b/js-api-spec/node-package-importer.node.test.ts index ac99bf15c0..83aa2d7529 100644 --- a/js-api-spec/node-package-importer.node.test.ts +++ b/js-api-spec/node-package-importer.node.test.ts @@ -453,16 +453,12 @@ describe('Node Package Importer', () => { 'node_modules/bar-abs/package.json': JSON.stringify({}), }); const entryPoint = fileURLToPath(dir.url()); - console.log(`pkg:bar-abs entrypoint- ${entryPoint}`); - return dir.chdir( - () => { - const result = compileString('@use "pkg:bar-abs";', { - importers: [new NodePackageImporter()], - }); - return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); - }, - {entryPoint} - ); + return dir.chdir(() => { + const result = compileString('@use "pkg:bar-abs";', { + importers: [new NodePackageImporter(entryPoint)], + }); + return expect(result.css).toEqualIgnoringWhitespace('a {b: c;}'); + }); })); it('resolves in scoped package', () =>