diff options
| -rw-r--r-- | .changeset/small-ties-sort.md | 5 | ||||
| -rw-r--r-- | packages/astro/src/runtime/server/render/util.ts | 21 | ||||
| -rw-r--r-- | packages/astro/test/astro-attrs.test.js | 38 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/astro-attrs/src/pages/index.astro | 35 | ||||
| -rw-r--r-- | packages/integrations/mdx/test/mdx-vite-env-vars.test.js | 4 |
5 files changed, 67 insertions, 36 deletions
diff --git a/.changeset/small-ties-sort.md b/.changeset/small-ties-sort.md new file mode 100644 index 000000000..2269d714d --- /dev/null +++ b/.changeset/small-ties-sort.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes attribute rendering for non-boolean attributes with boolean values diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index 469491de4..31a78a92e 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -8,9 +8,6 @@ export const voidElementNames = /^(area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/i; const htmlBooleanAttributes = /^(?:allowfullscreen|async|autofocus|autoplay|controls|default|defer|disabled|disablepictureinpicture|disableremoteplayback|formnovalidate|hidden|loop|nomodule|novalidate|open|playsinline|readonly|required|reversed|scoped|seamless|itemscope)$/i; -const htmlEnumAttributes = /^(?:contenteditable|draggable|spellcheck|value)$/i; -// Note: SVG is case-sensitive! -const svgEnumAttributes = /^(?:autoReverse|externalResourcesRequired|focusable|preserveAlpha)$/i; const AMPERSAND_REGEX = /&/g; const DOUBLE_QUOTE_REGEX = /"/g; @@ -67,13 +64,6 @@ export function addAttribute(value: any, key: string, shouldEscape = true) { return ''; } - if (value === false) { - if (htmlEnumAttributes.test(key) || svgEnumAttributes.test(key)) { - return markHTMLString(` ${key}="false"`); - } - return ''; - } - // compiler directives cannot be applied dynamically, log a warning and ignore. if (STATIC_DIRECTIVES.has(key)) { // eslint-disable-next-line no-console @@ -115,11 +105,16 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the } // Boolean values only need the key - if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) { + if (htmlBooleanAttributes.test(key)) { + return markHTMLString(value ? ` ${key}` : ''); + } + + // Other attributes with an empty string value can omit rendering the value + if (value === '') { return markHTMLString(` ${key}`); - } else { - return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`); } + + return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`); } // Adds support for `<Component {...value} /> diff --git a/packages/astro/test/astro-attrs.test.js b/packages/astro/test/astro-attrs.test.js index f4a014042..58d1ddbbb 100644 --- a/packages/astro/test/astro-attrs.test.js +++ b/packages/astro/test/astro-attrs.test.js @@ -16,21 +16,41 @@ describe('Attributes', async () => { const $ = cheerio.load(html); const attrs = { - 'false-str': { attribute: 'attr', value: 'false' }, - 'true-str': { attribute: 'attr', value: 'true' }, - false: { attribute: 'attr', value: undefined }, - true: { attribute: 'attr', value: 'true' }, - empty: { attribute: 'attr', value: '' }, + 'boolean-attr-true': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-false': { attribute: 'allowfullscreen', value: undefined }, + 'boolean-attr-string-truthy': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-string-falsy': { attribute: 'allowfullscreen', value: undefined }, + 'boolean-attr-number-truthy': { attribute: 'allowfullscreen', value: '' }, + 'boolean-attr-number-falsy': { attribute: 'allowfullscreen', value: undefined }, + 'data-attr-true': { attribute: 'data-foobar', value: 'true' }, + 'data-attr-false': { attribute: 'data-foobar', value: 'false' }, + 'data-attr-string-truthy': { attribute: 'data-foobar', value: 'foo' }, + 'data-attr-string-falsy': { attribute: 'data-foobar', value: '' }, + 'data-attr-number-truthy': { attribute: 'data-foobar', value: '1' }, + 'data-attr-number-falsy': { attribute: 'data-foobar', value: '0' }, + 'normal-attr-true': { attribute: 'foobar', value: 'true' }, + 'normal-attr-false': { attribute: 'foobar', value: 'false' }, + 'normal-attr-string-truthy': { attribute: 'foobar', value: 'foo' }, + 'normal-attr-string-falsy': { attribute: 'foobar', value: '' }, + 'normal-attr-number-truthy': { attribute: 'foobar', value: '1' }, + 'normal-attr-number-falsy': { attribute: 'foobar', value: '0' }, null: { attribute: 'attr', value: undefined }, undefined: { attribute: 'attr', value: undefined }, - 'html-boolean': { attribute: 'async', value: 'async' }, - 'html-boolean-true': { attribute: 'async', value: 'async' }, - 'html-boolean-false': { attribute: 'async', value: undefined }, 'html-enum': { attribute: 'draggable', value: 'true' }, 'html-enum-true': { attribute: 'draggable', value: 'true' }, 'html-enum-false': { attribute: 'draggable', value: 'false' }, }; + assert.ok(!/allowfullscreen=/.test(html), 'boolean attributes should not have values'); + assert.ok( + !/id="data-attr-string-falsy"\s+data-foobar=/.test(html), + "data attributes should not have values if it's an empty string" + ); + assert.ok( + !/id="normal-attr-string-falsy"\s+data-foobar=/.test(html), + "normal attributes should not have values if it's an empty string" + ); + // cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually assert.equal( html.includes('https://example.com/api/og?title=hello&description=somedescription'), @@ -46,7 +66,7 @@ describe('Attributes', async () => { for (const id of Object.keys(attrs)) { const { attribute, value } = attrs[id]; const attr = $(`#${id}`).attr(attribute); - assert.equal(attr, value); + assert.equal(attr, value, `Expected ${attribute} to be ${value} for #${id}`); } }); diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro index 7ac96635f..8f2576650 100644 --- a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro +++ b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro @@ -1,19 +1,30 @@ -<span id="false-str" attr="false" /> -<span id="true-str" attr="true" /> -<span id="true" attr={true} /> -<span id="false" attr={false} /> -<span id="empty" attr="" /> +<span id="boolean-attr-true" allowfullscreen={true} /> +<span id="boolean-attr-false" allowfullscreen={false} /> +<span id="boolean-attr-string-truthy" allowfullscreen={'foo'} /> +<span id="boolean-attr-string-falsy" allowfullscreen={''} /> +<span id="boolean-attr-number-truthy" allowfullscreen={1} /> +<span id="boolean-attr-number-falsy" allowfullscreen={0} /> + +<span id="data-attr-true" data-foobar={true} /> +<span id="data-attr-false" data-foobar={false} /> +<span id="data-attr-string-truthy" data-foobar={'foo'} /> +<span id="data-attr-string-falsy" data-foobar={''} /> +<span id="data-attr-number-truthy" data-foobar={1} /> +<span id="data-attr-number-falsy" data-foobar={0} /> + +<span id="normal-attr-true" foobar={true} /> +<span id="normal-attr-false" foobar={false} /> +<span id="normal-attr-string-truthy" foobar={'foo'} /> +<span id="normal-attr-string-falsy" foobar={''} /> +<span id="normal-attr-number-truthy" foobar={1} /> +<span id="normal-attr-number-falsy" foobar={0} /> + <span id="null" attr={null} /> <span id="undefined" attr={undefined} /> + <span id="url" attr={"https://example.com/api/og?title=hello&description=somedescription"}/> <span id="code" attr={"cmd: echo \"foo\" && echo \"bar\" > /tmp/hello.txt"} /> -<!-- - Per HTML spec, some attributes should be treated as booleans - These should always render <span async /> or <span /> (without a string value) ---> -<span id='html-boolean' async /> -<span id='html-boolean-true' async={true} /> -<span id='html-boolean-false' async={false} /> + <!-- Other attributes should be treated as string enums These should always render <span draggable="true" /> or <span draggable="false" /> diff --git a/packages/integrations/mdx/test/mdx-vite-env-vars.test.js b/packages/integrations/mdx/test/mdx-vite-env-vars.test.js index 4e32d12cb..00fcfaab0 100644 --- a/packages/integrations/mdx/test/mdx-vite-env-vars.test.js +++ b/packages/integrations/mdx/test/mdx-vite-env-vars.test.js @@ -57,8 +57,8 @@ describe('MDX - Vite env vars', () => { const dataAttrDump = document.querySelector('[data-env-dump]'); assert.notEqual(dataAttrDump, null); - assert.notEqual(dataAttrDump.getAttribute('data-env-prod'), null); - assert.equal(dataAttrDump.getAttribute('data-env-dev'), null); + assert.equal(dataAttrDump.getAttribute('data-env-prod'), 'true'); + assert.equal(dataAttrDump.getAttribute('data-env-dev'), 'false'); assert.equal(dataAttrDump.getAttribute('data-env-base-url'), '/'); assert.equal(dataAttrDump.getAttribute('data-env-mode'), 'production'); }); |