fix: properly handle scoped class injection with spread attributes (#3384)

* fix: properly handle scoped class injection when a spread attribute is present

* chore: update lockfile

* chore: revert lockfile

* chore: update compiler

* test: add spread scope test

* test: fix spread scoped test
This commit is contained in:
Nate Moore 2022-05-24 11:58:21 -05:00 committed by GitHub
parent ccaea99765
commit 296fff2cff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 83 additions and 18 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Fix long-standing bug where a `class` attribute inside of a spread prop will cause duplicate `class` attributes

View file

@ -77,7 +77,7 @@
"test:e2e:match": "playwright test -g" "test:e2e:match": "playwright test -g"
}, },
"dependencies": { "dependencies": {
"@astrojs/compiler": "^0.14.3", "@astrojs/compiler": "^0.15.0",
"@astrojs/language-server": "^0.13.4", "@astrojs/language-server": "^0.13.4",
"@astrojs/markdown-remark": "^0.9.4", "@astrojs/markdown-remark": "^0.9.4",
"@astrojs/prism": "0.4.1", "@astrojs/prism": "0.4.1",

View file

@ -297,7 +297,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
// as a string and the user is responsible for adding a script tag for the component definition. // as a string and the user is responsible for adding a script tag for the component definition.
if (!html && typeof Component === 'string') { if (!html && typeof Component === 'string') {
html = await renderAstroComponent( html = await renderAstroComponent(
await render`<${Component}${spreadAttributes(props)}${markHTMLString( await render`<${Component}${internalSpreadAttributes(props)}${markHTMLString(
(children == null || children == '') && voidElementNames.test(Component) (children == null || children == '') && voidElementNames.test(Component)
? `/>` ? `/>`
: `>${children == null ? '' : children}</${Component}>` : `>${children == null ? '' : children}</${Component}>`
@ -426,7 +426,7 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
} }
// Adds support for `<Component {...value} /> // Adds support for `<Component {...value} />
export function spreadAttributes(values: Record<any, any>, shouldEscape = true) { function internalSpreadAttributes(values: Record<any, any>, shouldEscape = true) {
let output = ''; let output = '';
for (const [key, value] of Object.entries(values)) { for (const [key, value] of Object.entries(values)) {
output += addAttribute(value, key, shouldEscape); output += addAttribute(value, key, shouldEscape);
@ -434,6 +434,25 @@ export function spreadAttributes(values: Record<any, any>, shouldEscape = true)
return markHTMLString(output); return markHTMLString(output);
} }
// Adds support for `<Component {...value} />
export function spreadAttributes(values: Record<any, any>, name: string, { class: scopedClassName }: { class?: string } = {}) {
let output = '';
// If the compiler passes along a scoped class, merge with existing props or inject it
if (scopedClassName) {
if (typeof values.class !== 'undefined') {
values.class += ` ${scopedClassName}`;
} else if (typeof values['class:list'] !== 'undefined') {
values['class:list'] = [values['class:list'], scopedClassName];
} else {
values.class = scopedClassName;
}
}
for (const [key, value] of Object.entries(values)) {
output += addAttribute(value, key, true);
}
return markHTMLString(output);
}
// Adds CSS variables to an inline style tag // Adds CSS variables to an inline style tag
export function defineStyleVars(selector: string, vars: Record<any, any>) { export function defineStyleVars(selector: string, vars: Record<any, any>) {
let output = '\n'; let output = '\n';
@ -692,5 +711,5 @@ function renderElement(
children = defineScriptVars(defineVars) + '\n' + children; children = defineScriptVars(defineVars) + '\n' + children;
} }
} }
return `<${name}${spreadAttributes(props, shouldEscape)}>${children}</${name}>`; return `<${name}${internalSpreadAttributes(props, shouldEscape)}>${children}</${name}>`;
} }

View file

@ -76,6 +76,20 @@ describe('Astro basics', () => {
expect($('#spread-ts').attr('c')).to.equal('2'); expect($('#spread-ts').attr('c')).to.equal('2');
}); });
it('Allows scoped classes with spread', async () => {
const html = await fixture.readFile('/spread-scope/index.html');
const $ = cheerio.load(html);
expect($('#spread-plain')).to.have.lengthOf(1);
expect($('#spread-plain').attr('class')).to.match(/astro-.*/);
expect($('#spread-class')).to.have.lengthOf(1);
expect($('#spread-class').attr('class')).to.match(/astro-.*/);
expect($('#spread-class-list')).to.have.lengthOf(1);
expect($('#spread-class-list').attr('class')).to.match(/astro-.*/);
});
it('Allows using the Fragment element to be used', async () => { it('Allows using the Fragment element to be used', async () => {
const html = await fixture.readFile('/fragment/index.html'); const html = await fixture.readFile('/fragment/index.html');
const $ = cheerio.load(html); const $ = cheerio.load(html);

View file

@ -0,0 +1,18 @@
---
const spread = { a: 0, b: 1, c: 2 };
---
<html>
<head>
<style>
div {
background-color: red;
}
</style>
</head>
<body>
<div id="spread-plain" {...spread} />
<div id="spread-class" {...spread} class="has-class" />
<div id="spread-class-list" {...spread} class:list={{ "has-class-list": true }} />
</body>
</html>

View file

@ -459,7 +459,7 @@ importers:
packages/astro: packages/astro:
specifiers: specifiers:
'@astrojs/compiler': ^0.14.3 '@astrojs/compiler': ^0.15.0
'@astrojs/language-server': ^0.13.4 '@astrojs/language-server': ^0.13.4
'@astrojs/markdown-remark': ^0.9.4 '@astrojs/markdown-remark': ^0.9.4
'@astrojs/prism': 0.4.1 '@astrojs/prism': 0.4.1
@ -545,7 +545,7 @@ importers:
yargs-parser: ^21.0.1 yargs-parser: ^21.0.1
zod: ^3.17.3 zod: ^3.17.3
dependencies: dependencies:
'@astrojs/compiler': 0.14.3 '@astrojs/compiler': 0.15.0
'@astrojs/language-server': 0.13.4 '@astrojs/language-server': 0.13.4
'@astrojs/markdown-remark': link:../markdown/remark '@astrojs/markdown-remark': link:../markdown/remark
'@astrojs/prism': link:../astro-prism '@astrojs/prism': link:../astro-prism
@ -2047,8 +2047,8 @@ packages:
leven: 3.1.0 leven: 3.1.0
dev: true dev: true
/@astrojs/compiler/0.14.3: /@astrojs/compiler/0.15.0:
resolution: {integrity: sha512-kG8E6rcy0rLJc24MrkoQ5THEPTVeVSTgOT/OmIK/K3UkywOWqs4D7w2MNvehR8pgLLzNVp0aen8+w57zVBFGew==} resolution: {integrity: sha512-gsw9OP/mfMIrdbrBaG5BpP98sWiAymeeVJTi365OwzDvOaePUzKqAMGCQd07RAfeX9eQzTkjDeJJZkIoyt575w==}
dependencies: dependencies:
tsm: 2.2.1 tsm: 2.2.1
uvu: 0.5.3 uvu: 0.5.3
@ -3829,7 +3829,7 @@ packages:
dependencies: dependencies:
'@lit-labs/ssr-client': 1.0.1 '@lit-labs/ssr-client': 1.0.1
'@lit/reactive-element': 1.3.2 '@lit/reactive-element': 1.3.2
'@types/node': 16.11.36 '@types/node': 16.11.35
lit: 2.2.4 lit: 2.2.4
lit-element: 3.2.0 lit-element: 3.2.0
lit-html: 2.2.4 lit-html: 2.2.4
@ -6473,8 +6473,8 @@ packages:
resolution: {integrity: sha512-B9EoJFjhqcQ9OmQrNorItO+OwEOORNn3S31WuiHvZY/dm9ajkB7AKD/8toessEtHHNL+58jofbq7hMMY9v4yig==} resolution: {integrity: sha512-B9EoJFjhqcQ9OmQrNorItO+OwEOORNn3S31WuiHvZY/dm9ajkB7AKD/8toessEtHHNL+58jofbq7hMMY9v4yig==}
dev: true dev: true
/@types/node/16.11.36: /@types/node/16.11.35:
resolution: {integrity: sha512-FR5QJe+TaoZ2GsMHkjuwoNabr+UrJNRr2HNOo+r/7vhcuntM6Ee/pRPOnRhhL2XE9OOvX9VLEq+BcXl3VjNoWA==} resolution: {integrity: sha512-QXu45LyepgnhUfnIAj/FyT4uM87ug5KpIrgXfQtUPNAlx8w5hmd8z8emqCLNvG11QkpRSCG9Qg2buMxvqfjfsQ==}
dev: false dev: false
/@types/node/17.0.35: /@types/node/17.0.35:
@ -6529,14 +6529,14 @@ packages:
dependencies: dependencies:
'@types/prop-types': 15.7.5 '@types/prop-types': 15.7.5
'@types/scheduler': 0.16.2 '@types/scheduler': 0.16.2
csstype: 3.1.0 csstype: 3.0.11
/@types/react/18.0.9: /@types/react/18.0.9:
resolution: {integrity: sha512-9bjbg1hJHUm4De19L1cHiW0Jvx3geel6Qczhjd0qY5VKVE2X5+x77YxAepuCwVh4vrgZJdgEJw48zrhRIeF4Nw==} resolution: {integrity: sha512-9bjbg1hJHUm4De19L1cHiW0Jvx3geel6Qczhjd0qY5VKVE2X5+x77YxAepuCwVh4vrgZJdgEJw48zrhRIeF4Nw==}
dependencies: dependencies:
'@types/prop-types': 15.7.5 '@types/prop-types': 15.7.5
'@types/scheduler': 0.16.2 '@types/scheduler': 0.16.2
csstype: 3.1.0 csstype: 3.0.11
dev: false dev: false
/@types/resolve/1.17.1: /@types/resolve/1.17.1:
@ -7820,8 +7820,8 @@ packages:
/csstype/2.6.20: /csstype/2.6.20:
resolution: {integrity: sha512-/WwNkdXfckNgw6S5R125rrW8ez139lBHWouiBvX8dfMFtcn6V81REDqnH7+CRpRipfYlyU1CmOnOxrmGcFOjeA==} resolution: {integrity: sha512-/WwNkdXfckNgw6S5R125rrW8ez139lBHWouiBvX8dfMFtcn6V81REDqnH7+CRpRipfYlyU1CmOnOxrmGcFOjeA==}
/csstype/3.1.0: /csstype/3.0.11:
resolution: {integrity: sha512-uX1KG+x9h5hIJsaKR9xHUeUraxf8IODOwq9JLNPq6BwB04a/xgpq3rcx47l5BZu5zBPlgD342tdke3Hom/nJRA==} resolution: {integrity: sha512-sa6P2wJ+CAbgyy4KFssIb/JNMLxFvKF1pCYCSXS8ZMuqZnMsrxqI2E5sPyoTpxoPU/gVZMzr2zjOfg8GIZOMsw==}
/csv-generate/3.4.3: /csv-generate/3.4.3:
resolution: {integrity: sha512-w/T+rqR0vwvHqWs/1ZyMDWtHHSJaN06klRqJXBEpDJaM/+dZkso0OKh1VcuuYvK3XM53KysVNq8Ko/epCK8wOw==} resolution: {integrity: sha512-w/T+rqR0vwvHqWs/1ZyMDWtHHSJaN06klRqJXBEpDJaM/+dZkso0OKh1VcuuYvK3XM53KysVNq8Ko/epCK8wOw==}
@ -7866,6 +7866,11 @@ packages:
/debug/3.2.7: /debug/3.2.7:
resolution: {integrity: sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==} resolution: {integrity: sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==}
peerDependencies:
supports-color: '*'
peerDependenciesMeta:
supports-color:
optional: true
dependencies: dependencies:
ms: 2.1.3 ms: 2.1.3
dev: false dev: false
@ -8774,7 +8779,7 @@ packages:
/filelist/1.0.4: /filelist/1.0.4:
resolution: {integrity: sha512-w1cEuf3S+DrLCQL7ET6kz+gmlJdbq9J7yXCSjK/OZCPA+qEN1WyF4ZAf0YYJa4/shHJra2t/d/r8SV4Ji+x+8Q==} resolution: {integrity: sha512-w1cEuf3S+DrLCQL7ET6kz+gmlJdbq9J7yXCSjK/OZCPA+qEN1WyF4ZAf0YYJa4/shHJra2t/d/r8SV4Ji+x+8Q==}
dependencies: dependencies:
minimatch: 5.1.0 minimatch: 5.0.1
dev: true dev: true
/fill-range/7.0.1: /fill-range/7.0.1:
@ -10578,8 +10583,8 @@ packages:
brace-expansion: 1.1.11 brace-expansion: 1.1.11
dev: true dev: true
/minimatch/5.1.0: /minimatch/5.0.1:
resolution: {integrity: sha512-9TPBGGak4nHfGZsPBohm9AWg6NoT7QTCehS3BIJABslyZbzxfV78QM2Y6+i741OPZIafFAaiiEMh5OyIrJPgtg==} resolution: {integrity: sha512-nLDxIFRyhDblz3qMuq+SoRZED4+miJ/G+tdDrjkkkRnjAsBexeGpgjLEQ0blJy7rHhR2b93rhQY4SvyWu9v03g==}
engines: {node: '>=10'} engines: {node: '>=10'}
dependencies: dependencies:
brace-expansion: 2.0.1 brace-expansion: 2.0.1
@ -10724,6 +10729,8 @@ packages:
debug: 3.2.7 debug: 3.2.7
iconv-lite: 0.4.24 iconv-lite: 0.4.24
sax: 1.2.4 sax: 1.2.4
transitivePeerDependencies:
- supports-color
dev: false dev: false
/netmask/2.0.2: /netmask/2.0.2:
@ -10807,6 +10814,8 @@ packages:
rimraf: 2.7.1 rimraf: 2.7.1
semver: 5.7.1 semver: 5.7.1
tar: 4.4.19 tar: 4.4.19
transitivePeerDependencies:
- supports-color
dev: false dev: false
/node-releases/2.0.5: /node-releases/2.0.5: