Skip to content

Commit

Permalink
🏗♻️ Enable babel-plugin-jsx-style-object in tests (#37562)
Browse files Browse the repository at this point in the history
Allows verifying the output of JSX elements whose style is set using an object.
  • Loading branch information
alanorozco committed Feb 8, 2022
1 parent 1ba0a82 commit 01df8ea
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 10 deletions.
1 change: 1 addition & 0 deletions build-system/babel-config/test-config.js
Expand Up @@ -53,6 +53,7 @@ function getTestConfig() {
argv.coverage ? instanbulPlugin : null,
replacePlugin,
replaceGlobalsPlugin,
'./build-system/babel-plugins/babel-plugin-jsx-style-object',
'./build-system/babel-plugins/babel-plugin-mangle-object-values',
'./build-system/babel-plugins/babel-plugin-imported-helpers',
'./build-system/babel-plugins/babel-plugin-transform-json-import',
Expand Down
Expand Up @@ -189,10 +189,7 @@ const renderInlineUi = (pageEl, attachmentEl) => {
return (
<div
class="i-amphtml-story-inline-page-attachment-img"
// TODO(alanorozco): This style attr would be nicer as an object.
// We need to enable babel-plugin-jsx-style-object in the testing config
// so that we can verify results of style objects.
style={`background-image: url(${proxied}) !important`}
style={{backgroundImage: `url(${proxied}) !important`}}
></div>
);
};
Expand Down
Expand Up @@ -161,7 +161,7 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
<div class="i-amphtml-amp-story-shopping-plp-card">
<div
class="i-amphtml-amp-story-shopping-plp-card-image"
style={`background-image: url("${data['productImages'][0]}")`}
style={{backgroundImage: `url("${data['productImages'][0]}")`}}
></div>
<div class="i-amphtml-amp-story-shopping-plp-card-brand">
{data['productBrand']}
Expand Down
4 changes: 2 additions & 2 deletions src/core/dom/jsx/index.ts
Expand Up @@ -14,8 +14,8 @@
* - Use standard HTML attribute names on elements (`class`, not `className`).
* - Dashes are ok, like `data-foo="bar"`.
*
* 🚫 No objects in attribute values, like `style={{width: 40}}`
* - For `style`, use strings instead.
* 🚫 No objects in attribute values, like `class={{foo: true, bar: false}}`
* - Objects in `style` are ok. They're converted to strings during build time.
* - For `class`, call `objstr()` or use strings instead.
*
* 🚫 No Fragments
Expand Down
19 changes: 16 additions & 3 deletions test/unit/core/dom/test-jsx.js
Expand Up @@ -148,6 +148,8 @@ describes.sandboxed('#core/dom/jsx', {}, (env) => {
});

it('renders with SVG namespace with props.xmlns', () => {
// Using `createElement` directly since `xmlns` is added during
// transformation by babel-plugin-dom-jsx-svg-namespace
const xmlns = 'http://www.w3.org/2000/svg';
const withProp = createElement('svg', {xmlns});
expect(withProp.namespaceURI).to.equal(xmlns);
Expand All @@ -156,6 +158,8 @@ describes.sandboxed('#core/dom/jsx', {}, (env) => {
});

it('renders with SVG namespace with props.xmlns (compiled)', () => {
// This works because <svg> is transformed by
// babel-plugin-dom-jsx-svg-namespace
const xmlns = 'http://www.w3.org/2000/svg';
const withProp = <svg />;
expect(withProp.namespaceURI).to.equal(xmlns);
Expand All @@ -171,14 +175,23 @@ describes.sandboxed('#core/dom/jsx', {}, (env) => {
});

it('does not support objects as attribute values', () => {
const element = (
<div style={{width: 400}} class={{foo: true, bar: false}} />
);
// Using `createElement` directly since objects in `style` JSXAttributes
// are otherwise transformed by babel-plugin-jsx-style-object
const element = createElement('div', {
style: {width: 400},
class: {foo: true, bar: false},
});
expect(element.outerHTML).to.equal(
'<div style="[object Object]" class="[object Object]"></div>'
);
});

it('supports object as style attribute value (compiled)', () => {
// This works because it's transformed by babel-plugin-jsx-style-object
const element = <div style={{width: 400, background: null}} />;
expect(element.outerHTML).to.equal('<div style="width:400px;"></div>');
});

it('does not support dangerouslySetInnerHTML', () => {
const element = <div dangerouslySetInnerHTML={{__html: '<script>'}} />;
expect(element.outerHTML).to.equal(
Expand Down

0 comments on commit 01df8ea

Please sign in to comment.