Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 39 additions & 27 deletions ember-scoped-css/src/build/template-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,41 +129,53 @@ export function createPlugin(config) {
/**
* We only allow a scoped <style> at the root
*/
let styleTag = node.body.find(
(n) => n.type === 'ElementNode' && n.tag === 'style',
let styleTags = node.body.filter(
(n) =>
n.type === 'ElementNode' &&
n.tag === 'style' &&
hasScopedAttribute(n),
);

if (hasScopedAttribute(styleTag)) {
let css = textContent(styleTag);
let lang = getLangAttribute(styleTag);
let info = getCSSContentInfo(css, lang);
if (styleTags.length > 1) {
throw new Error(
'Only one <style scoped> is supported per template, ' +
`but ${styleTags.length} were found. Merge them into one.`,
);
}

addInfo(info);
let styleTag = styleTags[0];

if (hasInlineAttributeWithoutLang(styleTag)) {
/**
* This will be handled in ElementNode traversal
*/
return;
}
if (!styleTag) return;

if (lang) {
/**
* For <style scoped inline lang="..."> we cannot preprocess at Babel-time
* (preprocessing is async and requires Vite's ResolvedConfig).
* Remove the tag and inject via virtual CSS module and warn user.
*/
console.warn(
`[ember-scoped-css] <style scoped inline lang="${lang}"> is not supported ` +
`(preprocessing is async and cannot run at Babel-time). ` +
`Downgrading to non-inline: the style tag will be removed and injected as a virtual CSS module.`,
);
}
let css = textContent(styleTag);
let lang = getLangAttribute(styleTag);
let info = getCSSContentInfo(css, lang);

let cssRequest = request.inline.create(info.id, postfix, css, lang);
addInfo(info);

env.meta.jsutils.importForSideEffect(cssRequest);
if (hasInlineAttributeWithoutLang(styleTag)) {
/**
* This will be handled in ElementNode traversal
*/
return;
}

if (lang) {
/**
* For <style scoped inline lang="..."> we cannot preprocess at Babel-time
* (preprocessing is async and requires Vite's ResolvedConfig).
* Remove the tag and inject via virtual CSS module and warn user.
*/
console.warn(
`[ember-scoped-css] <style scoped inline lang="${lang}"> is not supported ` +
`(preprocessing is async and cannot run at Babel-time). ` +
`Downgrading to non-inline: the style tag will be removed and injected as a virtual CSS module.`,
);
}

let cssRequest = request.inline.create(info.id, postfix, css, lang);

env.meta.jsutils.importForSideEffect(cssRequest);
},

// Visitors broken out like this so we can conditionally
Expand Down
51 changes: 51 additions & 0 deletions ember-scoped-css/src/build/template-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,54 @@ describe('lang attribute (SCSS preprocessor)', () => {
`);
});
});

/**
* https://github.com/auditboard/ember-scoped-css/issues/423
*/
describe('a scoped <style> is found regardless of what precedes it', () => {
it('extracts the scoped CSS when a global <style> comes first', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this test would be better as an inline snapshot. the individual expects lose context / "the plot", as claude often does

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(I think all the new tests, actually)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both new tests now assert with snapshots, in 8449956.

The template snapshot carries the whole story on its own -- scoped class postfixed, global block shipped as written, scoped tag removed:

[
  "<div class="scoped_e65d154a1">hi</div>
<style>
  .global { color: red; }
</style>",
]

plus the emitted virtual import, same shape as the lang="scss" tests above.

That made the third test ("leaves a preceding global <style>") a strict subset of this one, so I dropped it rather than keep a near-duplicate. Say the word if you would rather keep it separate.

The second-block case stayed a toThrow, matching babel-plugin.test.ts:99. Babel prefixes the message with the absolute filename, so toThrowErrorMatchingInlineSnapshot writes my home directory into the snapshot and only passes on this machine.

Both still fail against the pre-fix plugin -- I checked by swapping in the version from main.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Went back over the whole review. The four comment deletions were one note, not four, and I had only applied it to the tests -- template-plugin.js still carried the same kind of prose I had added above the filter and the length check. Both gone in cee6d1a, leaving the one-line comment that was there before this branch. The error message covers what the second one was saying.

The diff against main for that file is now just the behaviour change.

let output = await transform(`
export const Foo = <template>
<div class="scoped">hi</div>
<style>
.global { color: red; }
</style>
<style scoped>
.scoped { color: blue; }
</style>
</template>;
`);

expect(templateContentsOf(output)).toMatchInlineSnapshot(`
[
"<div class="scoped_e65d154a1">hi</div>
<style>
.global { color: red; }
</style>",
]
`);
expect(virtualImportUrlsOf(output)).toMatchInlineSnapshot(`
[
"./e65d154a1___css-68ede36d709bfa7f8a2994e1702ef010.ember-scoped.css?css=%0A%20%20%20%20%20%20%20%20%20%20.scoped%20%7B%20color%3A%20blue%3B%20%7D%0A%20%20%20%20%20%20%20%20",
]
`);
});

it('refuses a second <style scoped> rather than dropping its CSS', async () => {
let build = transform(`
export const Foo = <template>
<div class="first second">hi</div>
<style scoped>
.first { color: blue; }
</style>
<style scoped>
.second { color: green; }
</style>
</template>;
`);

await expect(build).rejects.toThrow(
/Only one <style scoped> is supported per template, but 2 were found/,
);
});
});
Loading