From e395b8869e8b75fec7b9211e99bdaaf7963a11c7 Mon Sep 17 00:00:00 2001 From: Ayush Sharma Date: Wed, 10 Jul 2019 22:54:32 +0200 Subject: [PATCH] Element recycling without attributes (#87) --- CHANGELOG.md | 1 + .../success/static_to_dynamic_attrs.template | 9 ++++++ .../__snapshots__/CompilerSpec.js.snap | 28 ++++++++++++++++++ .../__tests__/ConditionalRenderingSpec.ts | 29 +++++++++++++++++++ .../ConditionalRenderingSpec.ts.snap | 10 +++++++ packages/melody-idom/src/core.ts | 1 + 6 files changed, 78 insertions(+) create mode 100644 packages/melody-compiler/__tests__/__fixtures__/success/static_to_dynamic_attrs.template create mode 100644 packages/melody-idom/__tests__/__snapshots__/ConditionalRenderingSpec.ts.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f65ae2..4b41d4c 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Warn about usage of non-breaking space [#107](https://github.com/trivago/melody/pull/107) - Fixed bindings of dispatchCustomEvent in `melody-streams` [#117](https://github.com/trivago/melody/pull/117) - Fixed `combineRefs` unsubscription in `melody-streams` [#120](https://github.com/trivago/melody/pull/120) +- Melody sometimes removes classes from an element / element recycling without attributes [#118](https://github.com/trivago/melody/pull/118/files) ### Chore & Maintenance - Removes `Chai` and `Sinon` support, Migrates tests to use `Jest`'s matchers. [#103](https://github.com/trivago/melody/pull/103) diff --git a/packages/melody-compiler/__tests__/__fixtures__/success/static_to_dynamic_attrs.template b/packages/melody-compiler/__tests__/__fixtures__/success/static_to_dynamic_attrs.template new file mode 100644 index 0000000..855ad31 --- /dev/null +++ b/packages/melody-compiler/__tests__/__fixtures__/success/static_to_dynamic_attrs.template @@ -0,0 +1,9 @@ +
+ {% if foo %} +
+ {% else %} +
+ {% endif %} +
+ + diff --git a/packages/melody-compiler/__tests__/__snapshots__/CompilerSpec.js.snap b/packages/melody-compiler/__tests__/__snapshots__/CompilerSpec.js.snap index bf016ef..acf0174 100644 --- a/packages/melody-compiler/__tests__/__snapshots__/CompilerSpec.js.snap +++ b/packages/melody-compiler/__tests__/__snapshots__/CompilerSpec.js.snap @@ -2108,6 +2108,34 @@ export default function Spaceless(props) { " `; +exports[`Compiler should correctly transform static to dynamic attrs.template 1`] = ` +" +import { elementVoid, elementOpen, elementClose } from \\"melody-idom\\"; +export const _template = {}; +const _statics = [\\"class\\", \\"foo\\"]; + +_template.render = function (_context) { + elementOpen(\\"div\\", null, null); + + if (_context.foo) { + elementVoid(\\"div\\", \\"7*U2;JR\\", _statics); + } else { + elementVoid(\\"div\\", null, null, \\"class\\", \\"foo \\" + (_context.bar ? \\"bar\\" : \\"\\")); + } + + elementClose(\\"div\\"); +}; + +if (process.env.NODE_ENV !== \\"production\\") { + _template.displayName = \\"StaticToDynamicAttrs\\"; +} + +export default function StaticToDynamicAttrs(props) { + return _template.render(props); +} +" +`; + exports[`Compiler should correctly transform styles.template 1`] = ` " import { text, elementOpen, elementClose, elementOpenStart, elementOpenEnd, attr } from \\"melody-idom\\"; diff --git a/packages/melody-idom/__tests__/ConditionalRenderingSpec.ts b/packages/melody-idom/__tests__/ConditionalRenderingSpec.ts index 1416724..ff4d154 100644 --- a/packages/melody-idom/__tests__/ConditionalRenderingSpec.ts +++ b/packages/melody-idom/__tests__/ConditionalRenderingSpec.ts @@ -105,6 +105,35 @@ describe('conditional rendering', () => { }); }); + describe('with static attributes', () => { + const _statics = ["class", "foo"]; + function render(_context) { + elementOpen("div", null, null); + + if (_context.condition) { + elementVoid("div", "7*U2;JR", _statics); + } else { + elementVoid("div", null, null, "class", "foo " + (_context.bar ? "bar" : "")); + } + + elementClose("div"); + } + + it('should apply static attributes when recycling an element', () => { + patch(container, () => render({ condition: false, bar: true })); + expect(container.innerHTML).toMatchSnapshot(); + patch(container, () => render({ condition: true, bar: true })); + expect(container.innerHTML).toMatchSnapshot(); + }); + + it('should remove static attributes when recycling an element', () => { + patch(container, () => render({ condition: true, bar: true })); + expect(container.innerHTML).toMatchSnapshot(); + patch(container, () => render({ condition: false, bar: true })); + expect(container.innerHTML).toMatchSnapshot(); + }); + }); + describe('nodes', () => { function render(condition) { elementOpen('div', null, null, 'id', 'outer'); diff --git a/packages/melody-idom/__tests__/__snapshots__/ConditionalRenderingSpec.ts.snap b/packages/melody-idom/__tests__/__snapshots__/ConditionalRenderingSpec.ts.snap new file mode 100644 index 0000000..042c14a --- /dev/null +++ b/packages/melody-idom/__tests__/__snapshots__/ConditionalRenderingSpec.ts.snap @@ -0,0 +1,10 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`conditional rendering with static attributes should apply static attributes when recycling an element 1`] = `"
"`; + +exports[`conditional rendering with static attributes should apply static attributes when recycling an element 2`] = `"
"`; + +exports[`conditional rendering with static attributes should remove static attributes when recycling an element 1`] = `"
"`; + +exports[`conditional rendering with static attributes should remove static attributes when recycling an element 2`] = `"
"`; + diff --git a/packages/melody-idom/src/core.ts b/packages/melody-idom/src/core.ts index bbc1a58..15ce809 100644 --- a/packages/melody-idom/src/core.ts +++ b/packages/melody-idom/src/core.ts @@ -225,6 +225,7 @@ var matches = function( // which means we can hook onto it freely if (!data.key) { data.key = key; + data.staticsApplied = false; // but we'll need to update the parent element const parentKeys = currentParent && getData(currentParent).keyMap; if (parentKeys) {