feat: improve namespace readability and potential optimization confli…

…ct (#2928)

* fix: allow dash in namespace
* feat:  reduce multiple dashed and underscores into a single lodash or underscore
* feat: support non-ascii namespace from filename
* feat: validate st-namespace similarly to filename ns-resolve

packages/core/src/features/st-namespace.ts

@@ -1,7 +1,7 @@
 import path from 'path';
 import { createFeature, FeatureContext } from './feature';
 import { plugableRecord } from '../helpers/plugable-record';
-import { filename2varname } from '../helpers/string';
+import { filename2varname, string2varname } from '../helpers/string';
 import { stripQuotation } from '../helpers/string';
 import valueParser from 'postcss-value-parser';
 import { murmurhash3_32_gc } from '../murmurhash';
@@ -166,10 +166,10 @@ export function parseNamespace(node: AtRule, diag?: Diagnostics): string | undef
         });
         return;
     }
-    // ident like - without escapes
-    // eslint-disable-next-line no-control-regex
-    if (!namespace.match(/^([a-zA-Z-_]|[^\x00-\x7F]+)([a-zA-Z-_0-9]|[^\x00-\x7F])*$/)) {
-        // empty namespace found
+    // check namespace is a valid ident start with no conflicts with stylable namespacing
+    const transformedNamespace = string2varname(namespace);
+    if (namespace !== transformedNamespace) {
+        // invalid namespace found
         diag?.report(diagnostics.INVALID_NAMESPACE_VALUE(), {
             node,
             word: namespace,

packages/core/src/helpers/string.ts

@@ -1,11 +1,30 @@
+/* eslint-disable no-control-regex */
 export function stripQuotation(str: string) {
     return str.replace(/^['"](.*?)['"]$/g, '$1');
 }
 
 export function filename2varname(filename: string) {
-    return string2varname(filename.replace(/(?=.*)\.\w+$/, '').replace(/\.st$/, ''));
+    return string2varname(
+        filename
+            // remove extension (eg. .css)
+            .replace(/(?=.*)\.\w+$/, '')
+            // remove potential .st extension prefix
+            .replace(/\.st$/, '')
+    );
 }
 
-function string2varname(str: string) {
-    return str.replace(/[^0-9a-zA-Z_]/gm, '').replace(/^[^a-zA-Z_]+/gm, '');
+export function string2varname(str: string) {
+    return (
+        str
+            // allow only letters, numbers, dashes, underscores, and non-ascii
+            .replace(/[\x00-\x7F]+/gm, (matchAscii) => {
+                return matchAscii.replace(/[^0-9a-zA-Z_-]/gm, '');
+            })
+            // replace multiple dashes with single dash
+            .replace(/--+/gm, '-')
+            // replace multiple underscores with single underscore
+            .replace(/__+/gm, '_')
+            // remove leading digits from start
+            .replace(/^\d+/gm, '')
+    );
 }

packages/core/test/features/st-namespace.spec.ts

@@ -16,14 +16,34 @@ describe('features/st-namespace', () => {
     it('should use filename as default namespace', () => {
         const { sheets } = testStylableCore({
             '/a.st.css': ``,
-            '/b.st.css': ``,
+            '/B.st.css': ``,
+            '/-dash.st.css': ``,
+            '/_underscore.st.css': ``,
+            '/--multi---dash.st.css': ``,
+            '/__multi___underscore.st.css': ``,
+            '/🤡emoji🤷‍♀️why.st.css': ``,
+            '/123numbers-not-at-start789.st.css': ``,
         });
 
         const AMeta = sheets['/a.st.css'].meta;
-        const BMeta = sheets['/b.st.css'].meta;
+        const BMeta = sheets['/B.st.css'].meta;
+        const dashMeta = sheets['/-dash.st.css'].meta;
+        const underscoreMeta = sheets['/_underscore.st.css'].meta;
+        const multiDashMeta = sheets['/--multi---dash.st.css'].meta;
+        const multiUnderscoreMeta = sheets['/__multi___underscore.st.css'].meta;
+        const emojiMeta = sheets['/🤡emoji🤷‍♀️why.st.css'].meta;
+        const numbersMeta = sheets['/123numbers-not-at-start789.st.css'].meta;
 
         expect(AMeta.namespace, 'a meta.namespace').to.eql('a');
-        expect(BMeta.namespace, 'b meta.namespace').to.eql('b');
+        expect(BMeta.namespace, 'b meta.namespace').to.eql('B');
+        expect(dashMeta.namespace, '-dash meta.namespace').to.eql('-dash');
+        expect(underscoreMeta.namespace, '_underscore meta.namespace').to.eql('_underscore');
+        expect(multiDashMeta.namespace, '-multi-dash meta.namespace').to.eql('-multi-dash');
+        expect(multiUnderscoreMeta.namespace, '_multi_underscore meta.namespace').to.eql(
+            '_multi_underscore'
+        );
+        expect(emojiMeta.namespace, 'emoji meta.namespace').to.eql('🤡emoji🤷‍♀️why');
+        expect(numbersMeta.namespace, 'numbers meta.namespace').to.eql('numbers-not-at-start789');
     });
     it('should override default namespace with @st-namespace', () => {
         const { sheets } = testStylableCore({
@@ -134,9 +154,9 @@ describe('features/st-namespace', () => {
                 .x {}
             `,
             '/dash.st.css': `
-                @st-namespace "---";
+                @st-namespace "-";
 
-                /* x-@rule .\\---__x */
+                /* @rule .-__x */
                 .x {}
             `,
         });
@@ -155,7 +175,7 @@ describe('features/st-namespace', () => {
         expect(underscoreMeta.namespace, 'underscore namespace').to.eql('_x123_');
 
         shouldReportNoDiagnostics(dashMeta);
-        expect(dashMeta.namespace, 'dash namespace').to.eql('---');
+        expect(dashMeta.namespace, 'dash namespace').to.eql('-');
     });
     it('should report non string namespace', () => {
         const { sheets } = testStylableCore({
@@ -225,6 +245,18 @@ describe('features/st-namespace', () => {
                     @analyze-error(start with number) word(5abc) ${diagnostics.INVALID_NAMESPACE_VALUE()}
                 */
                 @st-namespace "5abc";
+
+                /* 
+                    @transform-remove
+                    @analyze-error(dash sequence) word(a--b) ${diagnostics.INVALID_NAMESPACE_VALUE()}
+                */
+                @st-namespace "a--b";
+
+                /* 
+                    @transform-remove
+                    @analyze-error(underscore sequence) word(a__b) ${diagnostics.INVALID_NAMESPACE_VALUE()}
+                */
+                @st-namespace "a__b";
             `,
         });
 

packages/esbuild/test/e2e/simple-case/simple-case.spec.ts

@@ -16,12 +16,12 @@ const stylesInOrder = [
     {
         path: 'side-effects.st.css',
         fileName: 'side-effects.st.css',
-        namespace: 'sideeffects',
+        namespace: 'side-effects',
     },
     {
         path: `internal-dir${sep}internal-dir.st.css`,
         fileName: 'internal-dir.st.css',
-        namespace: 'internaldir',
+        namespace: 'internal-dir',
     },
     {
         path: 'a.st.css',
@@ -115,9 +115,11 @@ async function contract(
                 ).getPropertyValue('--unused-deep'),
             };
         });
-        
+
     const assetLoaded = Boolean(responses?.find((r) => r.url().match(/asset-.*?\.png$/)));
-    const internalDirAsset = Boolean(responses?.find((r) => r.url().match(/internal-dir-.*?\.png$/)));
+    const internalDirAsset = Boolean(
+        responses?.find((r) => r.url().match(/internal-dir-.*?\.png$/))
+    );
 
     expect(internalDirAsset, 'asset loaded from internal dir').to.eql(true);
     expect(assetLoaded, 'asset loaded').to.eql(true);

packages/node/test/require-hook.spec.ts

@@ -65,7 +65,7 @@ describe('require hook', () => {
         attachHook({ ignoreJSModules: true });
         const m = require(join(fixturesPath, 'has-js.st.css'));
         expect(m.test).equal(undefined);
-        expect(m.namespace).to.match(/^hasjs/);
+        expect(m.namespace).to.match(/^has-js/);
     });
 
     it('should throw on missing config file', () => {

packages/rollup-plugin/test/rollup-side-effects.spec.ts

@@ -43,7 +43,7 @@ describe('StylableRollupPlugin - include all stylesheets with side-effects', fun
         @layer globalLayer;
         
         html {
-            --globalselector-x: green;
+            --global-selector-x: green;
         }
         .index__root {}
         `);

packages/webpack-extensions/test/e2e/metadata-loader-case.spec.ts

@@ -50,6 +50,6 @@ describe(`(${project})`, () => {
 
         expect(metadata.namespaceMapping[`/${index.hash}.st.css`]).to.match(/index\d+/);
         expect(metadata.namespaceMapping[`/${comp.hash}.st.css`]).to.match(/comp\d+/);
-        expect(metadata.namespaceMapping[`/${compX.hash}.st.css`]).to.match(/compx\d+/);
+        expect(metadata.namespaceMapping[`/${compX.hash}.st.css`]).to.match(/comp-x\d+/);
     });
 });

packages/webpack-plugin/test/e2e/dual-mode-esm.spec.ts

@@ -63,12 +63,12 @@ describe(`(${project})`, () => {
         expect(vanillaStylesNoRuntime).to.eql(stylableStylesNoRuntime);
 
         expect(normalizeNamespace(vanillaStyles)).to.eql([
-            { id: 'designsystem', depth: '1' },
+            { id: 'design-system', depth: '1' },
             { id: 'label', depth: '1' },
             { id: 'button', depth: '2' },
-            { id: 'basictheme', depth: '3' },
-            { id: 'labeltheme', depth: '4' },
-            { id: 'buttontheme', depth: '4' },
+            { id: 'basic-theme', depth: '3' },
+            { id: 'label-theme', depth: '4' },
+            { id: 'button-theme', depth: '4' },
         ]);
     });
 });