Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler): make style getter behavior consistent between default and native components #5144

Merged
merged 3 commits into from
Dec 11, 2023
Merged

fix(compiler): make style getter behavior consistent between default and native components #5144

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4159e2e
fix(compiler): make style getter behavior consistent between default …
christian-bromann Dec 8, 2023
4bf5be3
apply changes for multi mode styles
christian-bromann Dec 8, 2023
c65c6e8
add comments that order of ifs must match
christian-bromann Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/compiler/transformers/add-static-style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,20 @@ const getMultipleModeStyle = (
const styleLiteral = createStyleLiteral(cmp, style, commentOriginalSelector);
const propStr = ts.factory.createPropertyAssignment(style.modeName, styleLiteral);
styleModes.push(propStr);
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagIosStyle from './import-path.css';
// static get style() { return { ios: myTagIosStyle }; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
const propIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleIdentifier);
styleModes.push(propIdentifier);
} else if (Array.isArray(style.externalStyles) && style.externalStyles.length > 0) {
// import generated from @Component() styleUrls option
// import myTagIosStyle from './import-path.css';
// static get style() { return { ios: myTagIosStyle }; }
const styleUrlIdentifier = createStyleIdentifier(cmp, style);
const propUrlIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleUrlIdentifier);
styleModes.push(propUrlIdentifier);
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagIosStyle from './import-path.css';
// static get style() { return { ios: myTagIosStyle }; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
const propIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleIdentifier);
styleModes.push(propIdentifier);
}
});

Expand Down
26 changes: 13 additions & 13 deletions src/compiler/transformers/component-native/native-static-style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ const addMultipleModeStyleGetter = (
const styleLiteral = createStyleLiteral(cmp, style);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GH won't allow me to comment on a line higher than this one - can we add a comment before this conditional starts (above the if statement) that the order of the conditional must match add-static-style.ts#getSingleStyle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on this point, I was thinking it might make sense to circle back after the bugfix is out and factor out this logic into a helper function since it looks essentially the same in these two spots

const propStr = ts.factory.createPropertyAssignment(style.modeName, styleLiteral);
styleModes.push(propStr);
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagIosStyle from './import-path.css';
// static get style() { return { "ios": myTagIosStyle }; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
const propIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleIdentifier);
styleModes.push(propIdentifier);
} else if (Array.isArray(style.externalStyles) && style.externalStyles.length > 0) {
// import generated from @Component() styleUrls option
// import myTagIosStyle from './import-path.css';
// static get style() { return { "ios": myTagIosStyle }; }
const styleUrlIdentifier = createStyleIdentifier(cmp, style);
const propUrlIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleUrlIdentifier);
styleModes.push(propUrlIdentifier);
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagIosStyle from './import-path.css';
// static get style() { return { "ios": myTagIosStyle }; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
const propIdentifier = ts.factory.createPropertyAssignment(style.modeName, styleIdentifier);
styleModes.push(propIdentifier);
}
});

Expand All @@ -65,18 +65,18 @@ const addSingleStyleGetter = (
// static get style() { return "string"; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GH won't allow me to comment on a line higher than this one - can we add a comment before this conditional starts (above the if statement) that the order of the conditional must match add-static-style.ts#getSingleStyle?

const styleLiteral = createStyleLiteral(cmp, style);
classMembers.push(createStaticGetter('style', styleLiteral));
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagStyle from './import-path.css';
// static get style() { return myTagStyle; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
classMembers.push(createStaticGetter('style', styleIdentifier));
} else if (Array.isArray(style.externalStyles) && style.externalStyles.length > 0) {
// import generated from @Component() styleUrls option
// import myTagStyle from './import-path.css';
// static get style() { return myTagStyle; }
const styleUrlIdentifier = createStyleIdentifier(cmp, style);
classMembers.push(createStaticGetter('style', styleUrlIdentifier));
} else if (typeof style.styleIdentifier === 'string') {
// direct import already written in the source code
// import myTagStyle from './import-path.css';
// static get style() { return myTagStyle; }
const styleIdentifier = ts.factory.createIdentifier(style.styleIdentifier);
classMembers.push(createStaticGetter('style', styleIdentifier));
}
};

Expand Down
43 changes: 43 additions & 0 deletions src/compiler/transformers/test/native-component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,47 @@ describe('nativeComponentTransform', () => {
);
});
});

describe('static style property', () => {
it.each([
[`styleUrl: 'cmp-a.css'`, `import CmpAStyle0 from './cmp-a.css?tag=cmp-a';`, `CmpAStyle0`],
[
`styleUrls: ['cmp-a.css', 'cmp-b.css', 'cmp-a.css']`,
`import CmpAStyle0 from './cmp-b.css?tag=cmp-a';
import CmpAStyle1 from './cmp-a.css?tag=cmp-a';`,
`CmpAStyle0 + CmpAStyle1`,
],
[
`styleUrls: { ios: 'cmp-a.ios.css', md: 'cmp-a.md.css' }`,
`import CmpAIosStyle0 from './cmp-a.ios.css?tag=cmp-a&mode=ios';
import CmpAMdStyle0 from './cmp-a.md.css?tag=cmp-a&mode=md';`,
`{ ios: CmpAIosStyle0, md: CmpAMdStyle0 }`,
],
])('adds a static style property when %s', async (styleConfig, expectedImport, expectedStyleReturn) => {
const code = `
@Component({
tag: 'cmp-a',
${styleConfig}
}
export class CmpA {}
`;
const transformer = nativeComponentTransform(compilerCtx, transformOpts);
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);

expect(await formatCode(transpiledModule.outputText)).toContain(
await formatCode(`import { defineCustomElement as __stencil_defineCustomElement, HTMLElement } from '@stencil/core';
${expectedImport}
const CmpA = class extends HTMLElement {
constructor() {
super();
this.__registerHost();
}
static get style() {
return ${expectedStyleReturn};
}
};
`),
);
});
});
});