fix(compiler): support multiple styleUrls in components (#5090)

* fix(compiler): support multiple styleUrls in components * apply change also to native component transformer * add tests * prettier * add another unit test * remove special characters properly * prettier fix * split parameter off var name * fix build issue * update test * Revert "update test" This reverts commit a1ca049. * don't attach queryparams when compiling to CJS * prettier fix * add additional comments to address coupling of identifier generation * make getIdentifierFromResourceUrl more robust * remove import * prettier
ionic-team · Nov 30, 2023 · 54e52da · 54e52da
1 parent df46fdc
commit 54e52da
Show file tree

Hide file tree

Showing 8 changed files with 318 additions and 76 deletions.
diff --git a/src/compiler/transformers/add-static-style.ts b/src/compiler/transformers/add-static-style.ts
@@ -1,10 +1,10 @@
-import { dashToPascalCase, DEFAULT_STYLE_MODE } from '@utils';
+import { DEFAULT_STYLE_MODE } from '@utils';
 import ts from 'typescript';
 
 import type * as d from '../../declarations';
 import { scopeCss } from '../../utils/shadow-css';
 import { getScopeId } from '../style/scope-css';
-import { createStaticGetter } from './transform-utils';
+import { createStaticGetter, getIdentifierFromResourceUrl } from './transform-utils';
 
 /**
  * Adds static "style" getter within the class
@@ -91,7 +91,7 @@ const getMultipleModeStyle = (
       // import generated from @Component() styleUrls option
       // import myTagIosStyle from './import-path.css';
       // static get style() { return { ios: myTagIosStyle }; }
-      const styleUrlIdentifier = createStyleIdentifierFromUrl(cmp, style);
+      const styleUrlIdentifier = createStyleIdentifierFromUrl(style.externalStyles);
       const propUrlIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleUrlIdentifier);
       styleModes.push(propUrlIdentifier);
     }
@@ -118,7 +118,7 @@ const getSingleStyle = (cmp: d.ComponentCompilerMeta, style: d.StyleCompiler, co
     // import generated from @Component() styleUrls option
     // import myTagStyle from './import-path.css';
     // static get style() { return myTagStyle; }
-    return createStyleIdentifierFromUrl(cmp, style);
+    return createStyleIdentifierFromUrl(style.externalStyles);
   }
 
   return null;
@@ -134,16 +134,46 @@ const createStyleLiteral = (cmp: d.ComponentCompilerMeta, style: d.StyleCompiler
   return ts.factory.createStringLiteral(style.styleStr);
 };
 
-const createStyleIdentifierFromUrl = (cmp: d.ComponentCompilerMeta, style: d.StyleCompiler) => {
-  style.styleIdentifier = dashToPascalCase(cmp.tagName);
-  style.styleIdentifier = style.styleIdentifier.charAt(0).toLowerCase() + style.styleIdentifier.substring(1);
-
-  if (style.modeName !== DEFAULT_STYLE_MODE) {
-    style.styleIdentifier += dashToPascalCase(style.modeName);
+/**
+ * Creates an expression to be assigned to the `style` property of a component class. For example
+ * given the following component:
+ *
+ * ```ts
+ * @Component({
+ *  styleUrls: ['my-component.css', 'my-component.ios.css']
+ * })
+ * export class MyComponent {
+ *   // ...
+ * }
+ * ```
+ *
+ * it would generate the following expression:
+ *
+ * ```ts
+ * import _myComponentCssStyle from './my-component.css';
+ * import _myComponentIosCssStyle from './my-component.ios.css';
+ * export class MyComponent {
+ *   // ...
+ * }
+ * MyComponent.style = _myComponentCssStyle + _myComponentIosCssStyle;
+ * ```
+ *
+ * Note: style imports are made in [`createEsmStyleImport`](src/compiler/transformers/style-imports.ts).
+ *
+ * @param externalStyles a list of external styles to be applied the component
+ * @returns an assignment expression to be applied to the `style` property of a component class (e.g. `_myComponentCssStyle + _myComponentIosCssStyle` based on the example)
+ */
+export const createStyleIdentifierFromUrl = (
+  externalStyles: d.ExternalStyleCompiler[],
+): ts.Identifier | ts.BinaryExpression => {
+  if (externalStyles.length === 1) {
+    return ts.factory.createIdentifier(getIdentifierFromResourceUrl(externalStyles[0].absolutePath));
   }
 
-  style.styleIdentifier += 'Style';
-  style.externalStyles = [style.externalStyles[0]];
-
-  return ts.factory.createIdentifier(style.styleIdentifier);
+  const firstExternalStyle = externalStyles[0];
+  return ts.factory.createBinaryExpression(
+    createStyleIdentifierFromUrl([firstExternalStyle]),
+    ts.SyntaxKind.PlusToken,
+    createStyleIdentifierFromUrl(externalStyles.slice(1)),
+  );
 };
diff --git a/src/compiler/transformers/component-native/native-static-style.ts b/src/compiler/transformers/component-native/native-static-style.ts
@@ -1,9 +1,10 @@
-import { dashToPascalCase, DEFAULT_STYLE_MODE } from '@utils';
+import { DEFAULT_STYLE_MODE } from '@utils';
 import ts from 'typescript';
 
 import type * as d from '../../../declarations';
 import { scopeCss } from '../../../utils/shadow-css';
 import { getScopeId } from '../../style/scope-css';
+import { createStyleIdentifierFromUrl } from '../add-static-style';
 import { createStaticGetter } from '../transform-utils';
 
 export const addNativeStaticStyle = (classMembers: ts.ClassElement[], cmp: d.ComponentCompilerMeta) => {
@@ -43,7 +44,7 @@ const addMultipleModeStyleGetter = (
       // import generated from @Component() styleUrls option
       // import myTagIosStyle from './import-path.css';
       // static get style() { return { "ios": myTagIosStyle }; }
-      const styleUrlIdentifier = createStyleIdentifierFromUrl(cmp, style);
+      const styleUrlIdentifier = createStyleIdentifierFromUrl(style.externalStyles);
       const propUrlIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleUrlIdentifier);
       styleModes.push(propUrlIdentifier);
     }
@@ -74,7 +75,7 @@ const addSingleStyleGetter = (
     // import generated from @Component() styleUrls option
     // import myTagStyle from './import-path.css';
     // static get style() { return myTagStyle; }
-    const styleUrlIdentifier = createStyleIdentifierFromUrl(cmp, style);
+    const styleUrlIdentifier = createStyleIdentifierFromUrl(style.externalStyles);
     classMembers.push(createStaticGetter('style', styleUrlIdentifier));
   }
 };
@@ -88,17 +89,3 @@ const createStyleLiteral = (cmp: d.ComponentCompilerMeta, style: d.StyleCompiler
 
   return ts.factory.createStringLiteral(style.styleStr);
 };
-
-const createStyleIdentifierFromUrl = (cmp: d.ComponentCompilerMeta, style: d.StyleCompiler) => {
-  style.styleIdentifier = dashToPascalCase(cmp.tagName);
-  style.styleIdentifier = style.styleIdentifier.charAt(0).toLowerCase() + style.styleIdentifier.substring(1);
-
-  if (style.modeName !== DEFAULT_STYLE_MODE) {
-    style.styleIdentifier += dashToPascalCase(style.modeName);
-  }
-
-  style.styleIdentifier += 'Style';
-  style.externalStyles = [style.externalStyles[0]];
-
-  return ts.factory.createIdentifier(style.styleIdentifier);
-};
diff --git a/src/compiler/transformers/stencil-import-path.ts b/src/compiler/transformers/stencil-import-path.ts
@@ -15,9 +15,14 @@ import type { ImportData, ParsedImport, SerializeImportData } from '../../declar
  * @param data import data to be serialized
  * @param styleImportData an argument which controls whether the import data
  * will be added to the path (formatted as query params)
+ * @param moduleSystem the module system we compile to
  * @returns a formatted string
  */
-export const serializeImportPath = (data: SerializeImportData, styleImportData: string | undefined | null): string => {
+export const serializeImportPath = (
+  data: SerializeImportData,
+  styleImportData: string | undefined | null,
+  moduleSystem?: 'esm' | 'cjs',
+): string => {
   let p = data.importeePath;
 
   if (isString(p)) {
@@ -28,7 +33,7 @@ export const serializeImportPath = (data: SerializeImportData, styleImportData:
       p = './' + p;
     }
 
-    if (styleImportData === 'queryparams' || styleImportData === undefined) {
+    if (moduleSystem !== 'cjs' && (styleImportData === 'queryparams' || styleImportData === undefined)) {
       const paramData: ImportData = {};
       if (isString(data.tag)) {
         paramData.tag = data.tag;

diff --git a/src/compiler/transformers/style-imports.ts b/src/compiler/transformers/style-imports.ts
@@ -2,8 +2,37 @@ import ts from 'typescript';
 
 import type * as d from '../../declarations';
 import { serializeImportPath } from './stencil-import-path';
-import { retrieveTsModifiers } from './transform-utils';
-
+import { getIdentifierFromResourceUrl, retrieveTsModifiers } from './transform-utils';
+
+/**
+ * This function adds imports (either in ESM or CJS syntax) for styles that are
+ * imported from the component's styleUrls option. For example, if a component
+ * has the following:
+ *
+ * ```ts
+ * @Component({
+ *  styleUrls: ['my-component.css', 'my-component.ios.css']
+ * })
+ * export class MyComponent {
+ *   // ...
+ * }
+ * ```
+ *
+ * then this function will add the following import statement:
+ *
+ * ```ts
+ * import _myComponentCssStyle from './my-component.css';
+ * import _myComponentIosCssStyle from './my-component.ios.css';
+ * ```
+ *
+ * Note that import identifier are used in [`addStaticStyleGetterWithinClass`](src/compiler/transformers/add-static-style.ts)
+ * to attach them to a components static style property.
+ *
+ * @param transformOpts transform options configured for the current output target transpilation
+ * @param tsSourceFile the TypeScript source file that is being updated
+ * @param moduleFile component file to update
+ * @returns an updated source file with the added import statements
+ */
 export const updateStyleImports = (
   transformOpts: d.TransformOptions,
   tsSourceFile: ts.SourceFile,
@@ -17,6 +46,14 @@ export const updateStyleImports = (
   return updateEsmStyleImports(transformOpts, tsSourceFile, moduleFile);
 };
 
+/**
+ * Iterate over all components defined in given module, collect import
+ * statements to be added and update source file with them.
+ * @param transformOpts transform options configured for the current output target transpilation
+ * @param tsSourceFile the TypeScript source file that is being updated
+ * @param moduleFile component file to update
+ * @returns update source file with added import statements
+ */
 const updateEsmStyleImports = (
   transformOpts: d.TransformOptions,
   tsSourceFile: ts.SourceFile,
@@ -28,15 +65,12 @@ const updateEsmStyleImports = (
 
   moduleFile.cmps.forEach((cmp) => {
     cmp.styles.forEach((style) => {
+      updateSourceFile = true;
       if (typeof style.styleIdentifier === 'string') {
-        updateSourceFile = true;
-        if (style.externalStyles.length > 0) {
-          // add style imports built from @Component() styleUrl option
-          styleImports.push(createEsmStyleImport(transformOpts, tsSourceFile, cmp, style));
-        } else {
-          // update existing esm import of a style identifier
-          statements = updateEsmStyleImportPath(transformOpts, tsSourceFile, statements, cmp, style);
-        }
+        statements = updateEsmStyleImportPath(transformOpts, tsSourceFile, statements, cmp, style);
+      } else if (style.externalStyles.length > 0) {
+        // add style imports built from @Component() styleUrl option
+        styleImports.push(...createEsmStyleImport(transformOpts, tsSourceFile, cmp, style));
       }
     });
   });
@@ -92,16 +126,38 @@ const createEsmStyleImport = (
   cmp: d.ComponentCompilerMeta,
   style: d.StyleCompiler,
 ) => {
-  const importName = ts.factory.createIdentifier(style.styleIdentifier);
-  const importPath = getStyleImportPath(transformOpts, tsSourceFile, cmp, style, style.externalStyles[0].absolutePath);
-
-  return ts.factory.createImportDeclaration(
-    undefined,
-    ts.factory.createImportClause(false, importName, undefined),
-    ts.factory.createStringLiteral(importPath),
-  );
+  const imports: ts.ImportDeclaration[] = [];
+  for (const externalStyle of style.externalStyles) {
+    /**
+     * Add import statement for each style
+     * e.g. `const _ImportPathStyle = require('./import-path.css');`
+     *
+     * Attention: if you make changes to the import identifier (e.g. `_ImportPathStyle`),
+     * you also need to update the identifier in [`createStyleIdentifierFromUrl`](`src/compiler/transformers/add-static-style.ts`).
+     */
+    const importIdentifier = ts.factory.createIdentifier(getIdentifierFromResourceUrl(externalStyle.absolutePath));
+    const importPath = getStyleImportPath(transformOpts, tsSourceFile, cmp, style, externalStyle.absolutePath);
+
+    imports.push(
+      ts.factory.createImportDeclaration(
+        undefined,
+        ts.factory.createImportClause(false, importIdentifier, undefined),
+        ts.factory.createStringLiteral(importPath),
+      ),
+    );
+  }
+
+  return imports;
 };
 
+/**
+ * Iterate over all components defined in given module, collect require
+ * statements to be added and update source file with them.
+ * @param transformOpts transform options configured for the current output target transpilation
+ * @param tsSourceFile the TypeScript source file that is being updated
+ * @param moduleFile component file to update
+ * @returns update source file with added import statements
+ */
 const updateCjsStyleRequires = (
   transformOpts: d.TransformOptions,
   tsSourceFile: ts.SourceFile,
@@ -111,9 +167,9 @@ const updateCjsStyleRequires = (
 
   moduleFile.cmps.forEach((cmp) => {
     cmp.styles.forEach((style) => {
-      if (typeof style.styleIdentifier === 'string' && style.externalStyles.length > 0) {
+      if (style.externalStyles.length > 0) {
         // add style imports built from @Component() styleUrl option
-        styleRequires.push(createCjsStyleRequire(transformOpts, tsSourceFile, cmp, style));
+        styleRequires.push(...createCjsStyleRequire(transformOpts, tsSourceFile, cmp, style));
       }
     });
   });
@@ -131,27 +187,41 @@ const createCjsStyleRequire = (
   cmp: d.ComponentCompilerMeta,
   style: d.StyleCompiler,
 ) => {
-  const importName = ts.factory.createIdentifier(style.styleIdentifier);
-  const importPath = getStyleImportPath(transformOpts, tsSourceFile, cmp, style, style.externalStyles[0].absolutePath);
-
-  return ts.factory.createVariableStatement(
-    undefined,
-    ts.factory.createVariableDeclarationList(
-      [
-        ts.factory.createVariableDeclaration(
-          importName,
-          undefined,
-          undefined,
-          ts.factory.createCallExpression(
-            ts.factory.createIdentifier('require'),
-            [],
-            [ts.factory.createStringLiteral(importPath)],
-          ),
+  const imports: ts.VariableStatement[] = [];
+  for (const externalStyle of style.externalStyles) {
+    /**
+     * Add import statement for each style
+     * e.g. `import _ImportPathStyle from './import-path.css';`
+     *
+     * Attention: if you make changes to the import identifier (e.g. `_ImportPathStyle`),
+     * you also need to update the identifier in [`createStyleIdentifierFromUrl`](`src/compiler/transformers/add-static-style.ts`).
+     */
+    const importIdentifier = ts.factory.createIdentifier(getIdentifierFromResourceUrl(externalStyle.absolutePath));
+    const importPath = getStyleImportPath(transformOpts, tsSourceFile, cmp, style, externalStyle.absolutePath);
+
+    imports.push(
+      ts.factory.createVariableStatement(
+        undefined,
+        ts.factory.createVariableDeclarationList(
+          [
+            ts.factory.createVariableDeclaration(
+              importIdentifier,
+              undefined,
+              undefined,
+              ts.factory.createCallExpression(
+                ts.factory.createIdentifier('require'),
+                [],
+                [ts.factory.createStringLiteral(importPath)],
+              ),
+            ),
+          ],
+          ts.NodeFlags.Const,
         ),
-      ],
-      ts.NodeFlags.Const,
-    ),
-  );
+      ),
+    );
+  }
+
+  return imports;
 };
 
 const getStyleImportPath = (
@@ -168,5 +238,5 @@ const getStyleImportPath = (
     encapsulation: cmp.encapsulation,
     mode: style.modeName,
   };
-  return serializeImportPath(importData, transformOpts.styleImportData);
+  return serializeImportPath(importData, transformOpts.styleImportData, transformOpts.module);
 };