From a96c5d06ce9b9fd92c9862a01a34e134666ebcfc Mon Sep 17 00:00:00 2001 From: Ido Rosenthal Date: Mon, 20 Nov 2023 11:53:12 +0200 Subject: [PATCH 1/3] test: add failing test - stylable.config shouldn't override explicit cli argument --- packages/cli/test/cli.spec.ts | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/cli/test/cli.spec.ts b/packages/cli/test/cli.spec.ts index 3da9a9ae1..55f76f500 100644 --- a/packages/cli/test/cli.spec.ts +++ b/packages/cli/test/cli.spec.ts @@ -156,6 +156,70 @@ describe('Stylable Cli', function () { ).equal(resolveNamespace('style', join(tempDir.path, 'style.st.css'))); }); + it('should use the default to stylable.config resolveNamespace', () => { + populateDirectorySync(tempDir.path, { + 'package.json': `{"name": "test", "version": "0.0.0"}`, + 'stylable.config.js': ` + let c = 0; + module.exports = { + defaultConfig(fs) { + return { + resolveNamespace: () => 'config-ns-' + c++ + }; + }, + }; + + `, + 'style.st.css': `.root{color:red}`, + }); + + runCliSync(['--rootDir', tempDir.path, '--cjs']); + + const dirContent = loadDirSync(tempDir.path); + + expect( + evalStylableModule<{ namespace: string }>( + dirContent['style.st.css.js'], + 'style.st.css.js' + ).namespace + ).equal('config-ns-0'); + }); + + it('should use explicit namespaceResolver argument over stylable.config resolveNamespace', () => { + populateDirectorySync(tempDir.path, { + 'package.json': `{"name": "test", "version": "0.0.0"}`, + 'stylable.config.js': ` + let c = 0; + module.exports = { + defaultConfig(fs) { + return { + resolveNamespace: () => 'config-ns-' + c++ + }; + }, + }; + + `, + 'custom-ns-resolver.js': ` + let c = 0; + module.exports.resolveNamespace = function resolveNamespace() { + return 'custom-ns-' + c++; + } + `, + 'style.st.css': `.root{color:red}`, + }); + + runCliSync(['--rootDir', tempDir.path, '--nsr', './custom-ns-resolver.js', '--cjs']); + + const dirContent = loadDirSync(tempDir.path); + + expect( + evalStylableModule<{ namespace: string }>( + dirContent['style.st.css.js'], + 'style.st.css.js' + ).namespace + ).equal('custom-ns-0'); + }); + it('build .st.css source files with namespace reference', () => { populateDirectorySync(tempDir.path, { 'package.json': `{"name": "test", "version": "0.0.0"}`, From aec07a09f65424b87a809579a366636134465da0 Mon Sep 17 00:00:00 2001 From: Ido Rosenthal Date: Mon, 20 Nov 2023 11:56:45 +0200 Subject: [PATCH 2/3] fix: namespaceResolver cli argument should override stylable.config --- packages/cli/src/build-stylable.ts | 8 +++++--- packages/cli/src/cli.ts | 10 ++++++---- packages/cli/src/config/resolve-options.ts | 6 +++--- packages/cli/src/types.ts | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/build-stylable.ts b/packages/cli/src/build-stylable.ts index 5c03d9a09..c5aedeac2 100644 --- a/packages/cli/src/build-stylable.ts +++ b/packages/cli/src/build-stylable.ts @@ -7,7 +7,6 @@ import { createBuildIdentifier, createDefaultOptions, hasStylableCSSOutput, - NAMESPACE_RESOLVER_MODULE_REQUEST, } from './config/resolve-options'; import { DiagnosticsManager } from './diagnostics-manager'; import { createDefaultLogger, levels } from './logger'; @@ -51,7 +50,7 @@ export async function buildStylable( }), outputFiles = new Map(), requireModule = require, - resolveNamespace = requireModule(NAMESPACE_RESOLVER_MODULE_REQUEST).resolveNamespace, + resolveNamespace, configFilePath, watchOptions = {}, }: BuildStylableContext = {} @@ -92,10 +91,13 @@ export async function buildStylable( fileSystem, requireModule, projectRoot, - resolveNamespace, resolverCache, fileProcessorCache, ...config?.defaultConfig, + resolveNamespace: + resolveNamespace || + config?.defaultConfig?.resolveNamespace || + requireModule('@stylable/node').resolveNamespace, }); const { service, generatedFiles } = await build(buildOptions, { diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index f9520ee6a..b4057586c 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -16,9 +16,11 @@ async function main() { config, } = argv; const rootDir = resolve(argv.rootDir); - const { resolveNamespace } = require(require.resolve(namespaceResolver, { - paths: [rootDir], - })); + const explicitResolveNs = + namespaceResolver && + require(require.resolve(namespaceResolver, { + paths: [rootDir], + })); // const log = createLogger( @@ -42,7 +44,7 @@ async function main() { overrideBuildOptions, defaultOptions, fs, - resolveNamespace, + resolveNamespace: explicitResolveNs?.resolveNamespace, watch, log, configFilePath: config, diff --git a/packages/cli/src/config/resolve-options.ts b/packages/cli/src/config/resolve-options.ts index 0b91d3e99..5908a6280 100644 --- a/packages/cli/src/config/resolve-options.ts +++ b/packages/cli/src/config/resolve-options.ts @@ -7,8 +7,6 @@ import type { CliArguments, BuildOptions, PartialBuildOptions } from '../types'; const { join } = nodeFs; -export const NAMESPACE_RESOLVER_MODULE_REQUEST = '@stylable/node'; - export function getCliArguments(): Arguments { const defaults = createDefaultOptions(); return yargs @@ -85,10 +83,12 @@ export function getCliArguments(): Arguments { alias: 'unsr', }) .option('namespaceResolver', { + type: 'string', description: 'node request to a module that exports a stylable resolveNamespace function', alias: 'nsr', - default: NAMESPACE_RESOLVER_MODULE_REQUEST, + defaultDescription: + 'default to @stylable/node, if stylable.config resolveNamespace is undefined', }) .option('injectCSSRequest', { type: 'boolean', diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index 97486edf9..170c4bb1a 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -102,7 +102,7 @@ export interface CliArguments { bundle: string | undefined; dtsSourceMap: boolean | undefined; useNamespaceReference: boolean | undefined; - namespaceResolver: string; + namespaceResolver?: string | undefined; injectCSSRequest: boolean | undefined; cssFilename: string | undefined; cssInJs: boolean | undefined; From f3b1f5915b055cc0d05ca20518505cbe249fb650 Mon Sep 17 00:00:00 2001 From: Ido Rosenthal Date: Mon, 20 Nov 2023 12:14:56 +0200 Subject: [PATCH 3/3] test:unified tests --- packages/cli/test/cli.spec.ts | 128 +++++++++++++++------------------- 1 file changed, 57 insertions(+), 71 deletions(-) diff --git a/packages/cli/test/cli.spec.ts b/packages/cli/test/cli.spec.ts index 55f76f500..6624b1af2 100644 --- a/packages/cli/test/cli.spec.ts +++ b/packages/cli/test/cli.spec.ts @@ -9,6 +9,7 @@ import { createTempDirectory, ITempDirectory, } from '@stylable/e2e-test-kit'; +import { nodeFs } from '@file-services/node'; import { STImport, STVar } from '@stylable/core/dist/features'; import { diagnosticBankReportToStrings } from '@stylable/core-test-kit'; @@ -137,87 +138,72 @@ describe('Stylable Cli', function () { ]); }); - it('single file build with default ns-resolver', () => { + it('should resolve resolveNamespace with defaults: cli-arg -> stylable.config -> default', () => { populateDirectorySync(tempDir.path, { 'package.json': `{"name": "test", "version": "0.0.0"}`, 'style.st.css': `.root{color:red}`, }); - const nsr = require.resolve('@stylable/node'); - runCliSync(['--rootDir', tempDir.path, '--nsr', nsr, '--cjs']); + { + // default + runCliSync(['--rootDir', tempDir.path, '--cjs']); - const dirContent = loadDirSync(tempDir.path); - - expect( - evalStylableModule<{ namespace: string }>( - dirContent['style.st.css.js'], - 'style.st.css.js' - ).namespace - ).equal(resolveNamespace('style', join(tempDir.path, 'style.st.css'))); - }); - - it('should use the default to stylable.config resolveNamespace', () => { - populateDirectorySync(tempDir.path, { - 'package.json': `{"name": "test", "version": "0.0.0"}`, - 'stylable.config.js': ` - let c = 0; - module.exports = { - defaultConfig(fs) { - return { - resolveNamespace: () => 'config-ns-' + c++ - }; - }, - }; - - `, - 'style.st.css': `.root{color:red}`, - }); - - runCliSync(['--rootDir', tempDir.path, '--cjs']); - - const dirContent = loadDirSync(tempDir.path); - - expect( - evalStylableModule<{ namespace: string }>( - dirContent['style.st.css.js'], - 'style.st.css.js' - ).namespace - ).equal('config-ns-0'); - }); + expect( + evalStylableModule<{ namespace: string }>( + loadDirSync(tempDir.path)['style.st.css.js'], + 'style.st.css.js' + ).namespace, + 'default' + ).equal(resolveNamespace('style', join(tempDir.path, 'style.st.css'))); + } + { + // stylable.config + nodeFs.writeFileSync( + join(tempDir.path, 'stylable.config.js'), + ` + let c = 0; + module.exports = { + defaultConfig(fs) { + return { + resolveNamespace: () => 'config-ns-' + c++ + }; + }, + }; + ` + ); - it('should use explicit namespaceResolver argument over stylable.config resolveNamespace', () => { - populateDirectorySync(tempDir.path, { - 'package.json': `{"name": "test", "version": "0.0.0"}`, - 'stylable.config.js': ` - let c = 0; - module.exports = { - defaultConfig(fs) { - return { - resolveNamespace: () => 'config-ns-' + c++ - }; - }, - }; - - `, - 'custom-ns-resolver.js': ` - let c = 0; - module.exports.resolveNamespace = function resolveNamespace() { - return 'custom-ns-' + c++; - } - `, - 'style.st.css': `.root{color:red}`, - }); + runCliSync(['--rootDir', tempDir.path, '--cjs']); - runCliSync(['--rootDir', tempDir.path, '--nsr', './custom-ns-resolver.js', '--cjs']); + expect( + evalStylableModule<{ namespace: string }>( + loadDirSync(tempDir.path)['style.st.css.js'], + 'style.st.css.js' + ).namespace, + 'stylable.config' + ).equal('config-ns-0'); + } + { + // cli argument + nodeFs.writeFileSync( + join(tempDir.path, 'custom-ns-resolver.js'), + ` + let c = 0; + module.exports.resolveNamespace = function resolveNamespace() { + return 'custom-ns-' + c++; + } + ` + ); - const dirContent = loadDirSync(tempDir.path); + runCliSync(['--rootDir', tempDir.path, '--nsr', './custom-ns-resolver.js', '--cjs']); - expect( - evalStylableModule<{ namespace: string }>( - dirContent['style.st.css.js'], - 'style.st.css.js' - ).namespace - ).equal('custom-ns-0'); + expect( + evalStylableModule<{ namespace: string }>( + loadDirSync(tempDir.path)['style.st.css.js'], + 'style.st.css.js' + ).namespace, + 'cli argument' + ).equal('custom-ns-0'); + } }); it('build .st.css source files with namespace reference', () => {