From 491086e90833b87066aeff3487f1b711a7d820f5 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Wed, 4 Jan 2023 16:53:51 -0500 Subject: [PATCH 1/7] implementing fine grained observability --- src/jsx-loader.js | 49 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/src/jsx-loader.js b/src/jsx-loader.js index 370b945..0e16fe2 100644 --- a/src/jsx-loader.js +++ b/src/jsx-loader.js @@ -117,7 +117,8 @@ function parseJsxElement(element, moduleContents = '') { if (left.object.type === 'ThisExpression') { if (left.property.type === 'Identifier') { // very naive (fine grained?) reactivity - string += ` ${name}="__this__.${left.property.name}${expression.operator}${right.raw}; __this__.render();"`; + // string += ` ${name}="__this__.${left.property.name}${expression.operator}${right.raw}; __this__.update(\\'${left.property.name}\\', null, __this__.${left.property.name});"`; + string += ` ${name}="__this__.${left.property.name}${expression.operator}${right.raw}; __this__.setAttribute(\\'${left.property.name}\\', __this__.${left.property.name});"`; } } } @@ -145,6 +146,9 @@ function parseJsxElement(element, moduleContents = '') { } } } + + // TODO make sure this only applies to `this` references! + string += ` data-wcc-${expression.name}="${name}" data-wcc-ins="attr"`; } } else { // xxx > @@ -171,6 +175,11 @@ function parseJsxElement(element, moduleContents = '') { if (type === 'Identifier') { // You have {count} TODOs left to complete + const { name } = element.expression; + + string = `${string.slice(0, string.lastIndexOf('>'))} data-wcc-${name}="\${this.${name}}" data-wcc-ins="text">`; + // TODO be able to remove this extra data attribute + // string = `${string.slice(0, string.lastIndexOf('>'))} data-wcc-${name} data-wcc-ins="text">`; string += `\$\{${element.expression.name}\}`; } else if (type === 'MemberExpression') { const { object } = element.expression.object; @@ -333,6 +342,9 @@ export function parseJsx(moduleURL) { } attributeChangedCallback(name, oldValue, newValue) { + console.debug('???attributeChangedCallback', { name }); + console.debug('???attributeChangedCallback', { oldValue }); + console.debug('???attributeChangedCallback', { newValue }); function getValue(value) { return value.charAt(0) === '{' || value.charAt(0) === '[' ? JSON.parse(value) @@ -352,11 +364,42 @@ export function parseJsx(moduleURL) { `; }).join('\n')} } - - this.render(); + this.update(name, oldValue, newValue); } } + update(name, oldValue, newValue) { + console.debug('Update tracking against....', this.constructor.observedAttributes); + console.debug('Updating', name); + console.debug('Swap old', oldValue); + console.debug('For new', newValue); + console.debug('this[name]', this[name]); + const attr = \`data-wcc-\${name}\`; + const selector = \`[\${attr}]\`; + console.debug({ attr }); + console.debug({ selector }); + + this.querySelectorAll(selector).forEach((el) => { + const needle = oldValue || el.getAttribute(attr); + console.debug({ el }) + console.debug({ needle }); + console.debug({ newValue }); + switch(el.getAttribute('data-wcc-ins')) { + case 'text': + el.textContent = el.textContent.replace(needle, newValue); + break; + case 'attr': + console.debug(el.hasAttribute(attr)) + if (el.hasAttribute(el.getAttribute(attr))) { + el.setAttribute(el.getAttribute(attr), newValue); + } + break; + } + }) + + console.debug('****************************'); + } + ${newModuleContents.slice(insertPoint)} `; /* eslint-enable indent */ From 4a7509bd08536df017b115a3fb97dc6975ca96fb Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Sun, 8 Jan 2023 15:18:25 -0500 Subject: [PATCH 2/7] naive implementation of derived attribute tracking --- src/jsx-loader.js | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/jsx-loader.js b/src/jsx-loader.js index 0e16fe2..c374503 100644 --- a/src/jsx-loader.js +++ b/src/jsx-loader.js @@ -226,10 +226,16 @@ function findThisReferences(context, statement) { // const { description } = this.todo; references.push(init.property.name); } else if (init.type === 'ThisExpression' && id && id.properties) { - // const { description } = this.todo; + // const { id, description } = this; id.properties.forEach((property) => { references.push(property.key.name); }); + } else { + // TODO we are just blindly tracking anything here. + // everything should ideally be mapped to actual this references, to create a strong chain of direct reactivity + // instead of tracking any declaration as a derived tracking attr + // for convenience here, we push the entire declaration here, instead of the name like for direct this references (see above) + references.push(declaration); } }); } @@ -333,12 +339,34 @@ export function parseJsx(moduleURL) { } let newModuleContents = escodegen.generate(tree); + const trackingAttrs = observedAttributes.filter(attr => typeof attr === 'string'); + // TODO ideally derivedAttrs would explicitely reference trackingAttrs + // and if there are no derivedAttrs, do not include the derivedGetters / derivedSetters code in the compiled output + const derivedAttrs = observedAttributes.filter(attr => typeof attr !== 'string'); + const derivedGetters = derivedAttrs.map(attr => { + return ` + get_${attr.id.name}(${trackingAttrs.join(',')}) { + console.log('@@@@@@@@@@@@@@@@@@@@ updating derivative value for => ${attr.id.name}'); + console.log('@@@@@@@@@@@@@@@@@@@@ new derivative value is =>', ${moduleContents.slice(attr.init.start, attr.init.end)}); + return ${moduleContents.slice(attr.init.start, attr.init.end)} + } + `; + }).join('\n'); + const derivedSetters = derivedAttrs.map(attr => { + const name = attr.id.name; + + return ` + const old_${name} = this.get_${name}(oldValue); + const new_${name} = this.get_${name}(newValue); + this.update('${name}', old_${name}, new_${name}); + `; + }).join('\n'); // TODO better way to determine value type? /* eslint-disable indent */ newModuleContents = `${newModuleContents.slice(0, insertPoint)} static get observedAttributes() { - return [${[...observedAttributes].map(attr => `'${attr}'`).join(',')}] + return [${[...trackingAttrs].map(attr => `'${attr}'`).join()}] } attributeChangedCallback(name, oldValue, newValue) { @@ -356,7 +384,7 @@ export function parseJsx(moduleURL) { } if (newValue !== oldValue) { switch(name) { - ${observedAttributes.map((attr) => { + ${trackingAttrs.map((attr) => { return ` case '${attr}': this.${attr} = getValue(newValue); @@ -389,7 +417,6 @@ export function parseJsx(moduleURL) { el.textContent = el.textContent.replace(needle, newValue); break; case 'attr': - console.debug(el.hasAttribute(attr)) if (el.hasAttribute(el.getAttribute(attr))) { el.setAttribute(el.getAttribute(attr), newValue); } @@ -397,9 +424,14 @@ export function parseJsx(moduleURL) { } }) + if ([${[...trackingAttrs].map(attr => `'${attr}'`).join()}].includes(name)) { + ${derivedSetters} + } console.debug('****************************'); } + ${derivedGetters} + ${newModuleContents.slice(insertPoint)} `; /* eslint-enable indent */ From 83ea8f17b1d912b0c12194a3fb58d7654c3697ff Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 8 Jan 2024 21:16:53 -0500 Subject: [PATCH 3/7] track and document caveats with inferredObservability targeting --- docs/pages/docs.md | 12 ++--- sandbox/components/counter-dsd.jsx | 2 +- .../fixtures/attribute-changed-callback.txt | 52 +++++++++++++++---- test/cases/jsx-shadow-dom/src/heading.jsx | 2 +- test/cases/jsx/jsx.spec.js | 3 +- test/cases/jsx/src/badge.jsx | 2 +- 6 files changed, 52 insertions(+), 21 deletions(-) diff --git a/docs/pages/docs.md b/docs/pages/docs.md index d4434b4..8e7a9c2 100644 --- a/docs/pages/docs.md +++ b/docs/pages/docs.md @@ -317,9 +317,9 @@ customElements.define('wcc-counter', Counter); ### (Inferred) Attribute Observability -An optional feature supported by JSX based compilation is a feature called `inferredObservability`. With this enabled, WCC will read any `this` member references in your component's `render` function and map each member instance to -- an entry in the `observedAttributes` array -- automatically handle `attributeChangedCallback` update (by calling `this.render()`) +An optional feature supported by JSX based compilation is `inferredObservability`. With this enabled, WCC will read any `this` member references in your component's `render` function and map each member instance to +1. an entry in the `observedAttributes` array +1. automatically handle `attributeChangedCallback` updates So taking the above counter example, and opting in to this feature, we just need to enable the `inferredObservability` option in the component ```jsx @@ -334,6 +334,7 @@ export default class Counter extends HTMLElement { return (
+ You have clicked {count} times
@@ -348,6 +349,5 @@ And so now when the attribute is set on this component, the component will re-re ``` Some notes / limitations: -- Please be aware of the above linked discussion which is tracking known bugs / feature requests / open items related to all things WCC + JSX. -- We consider the capability of this observability to be "coarse grained" at this time since WCC just re-runs the entire `render` function, replacing of the `innerHTML` for the host component. Thought it is still WIP, we are exploring a more ["fine grained" approach](https://github.com/ProjectEvergreen/wcc/issues/108) that will more efficient than blowing away all the HTML, a la in the style of [**lit-html**](https://lit.dev/docs/templates/overview/) or [**Solid**'s Signals](https://www.solidjs.com/tutorial/introduction_signals). -- This automatically _reflects properties used in the `render` function to attributes_, so YMMV. +- Please be aware of the above linked discussion and issue filter which is tracking any known bugs / feature requests / open items related to all things WCC + JSX. +- This automatically reflects properties used in the `render` function to attributes, so [YMMV](https://dictionary.cambridge.org/us/dictionary/english/ymmv). diff --git a/sandbox/components/counter-dsd.jsx b/sandbox/components/counter-dsd.jsx index 26c7a53..6f1d230 100644 --- a/sandbox/components/counter-dsd.jsx +++ b/sandbox/components/counter-dsd.jsx @@ -4,7 +4,7 @@ export default class CounterDsdJsx extends HTMLElement { connectedCallback() { if (!this.shadowRoot) { console.warn('NO shadowRoot detected for counter-dsd.jsx!'); - this.count = this.getAttribute('count') || 0; + this.count = parseInt(this.getAttribute('count'), 10) || 0; // having an attachShadow call is required for DSD this.attachShadow({ mode: 'open' }); diff --git a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt index 3801a39..87fb721 100644 --- a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt +++ b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt @@ -1,13 +1,43 @@ attributeChangedCallback(name, oldValue, newValue) { - function getValue(value) { - return value.charAt(0) === '{' || value.charAt(0) === '[' ? JSON.parse(value) : !isNaN(value) ? parseInt(value, 10) : value === 'true' || value === 'false' ? value === 'true' ? true : false : value; - } - if (newValue !== oldValue) { - switch (name) { - case 'count': - this.count = getValue(newValue); - break; - } - this.render(); - } + function getValue(value) { + return value.charAt(0) === '{' || value.charAt(0) === '[' ? JSON.parse(value) : !isNaN(value) ? parseInt(value, 10) : value === 'true' || value === 'false' ? value === 'true' ? true : false : value; + } + if (newValue !== oldValue) { + switch (name) { + case 'count': + this.count = getValue(newValue); + break; + } + this.update(name, oldValue, newValue); + } +} +update(name, oldValue, newValue) { + console.debug('Update tracking against....', this.constructor.observedAttributes); + console.debug('Updating', name); + console.debug('Swap old', oldValue); + console.debug('For new', newValue); + console.debug('this[name]', this[name]); + const attr = `data-wcc-${ name }`; + const selector = `[${ attr }]`; + console.debug({ attr }); + console.debug({ selector }); + this.querySelectorAll(selector).forEach(el => { + const needle = oldValue || el.getAttribute(attr); + console.debug({ el }); + console.debug({ needle }); + console.debug({ newValue }); + switch (el.getAttribute('data-wcc-ins')) { + case 'text': + el.textContent = el.textContent.replace(needle, newValue); + break; + case 'attr': + if (el.hasAttribute(el.getAttribute(attr))) { + el.setAttribute(el.getAttribute(attr), newValue); + } + break; + } + }); + if (['count'].includes(name)) { + } + console.debug('****************************'); } \ No newline at end of file diff --git a/test/cases/jsx-shadow-dom/src/heading.jsx b/test/cases/jsx-shadow-dom/src/heading.jsx index 68f6d1b..cd97a2b 100644 --- a/test/cases/jsx-shadow-dom/src/heading.jsx +++ b/test/cases/jsx-shadow-dom/src/heading.jsx @@ -17,7 +17,7 @@ export default class HeadingComponent extends HTMLElement { return (
-

Hello, {greeting}!

+

Hello, {greeting}!

); diff --git a/test/cases/jsx/jsx.spec.js b/test/cases/jsx/jsx.spec.js index 981e429..f22686e 100644 --- a/test/cases/jsx/jsx.spec.js +++ b/test/cases/jsx/jsx.spec.js @@ -24,6 +24,7 @@ describe('Run WCC For ', function() { before(async function() { const { html, metadata } = await renderToString(new URL('./src/counter.jsx', import.meta.url)); + console.log({ html }); meta = metadata; dom = new JSDOM(html); }); @@ -69,7 +70,7 @@ describe('Run WCC For ', function() { it('should handle an assignment expression with implicit reactivity using this.render', () => { const element = Array.from(buttons).find(button => button.getAttribute('id') === 'evt-assignment'); - expect(element.getAttribute('onclick')).to.be.equal('this.parentElement.parentElement.count-=1; this.parentElement.parentElement.render();'); + expect(element.getAttribute('onclick')).to.be.equal('this.parentElement.parentElement.count-=1; this.parentElement.parentElement.setAttribute(\'count\', this.parentElement.parentElement.count);'); }); }); diff --git a/test/cases/jsx/src/badge.jsx b/test/cases/jsx/src/badge.jsx index 4251609..8f9bd08 100644 --- a/test/cases/jsx/src/badge.jsx +++ b/test/cases/jsx/src/badge.jsx @@ -33,7 +33,7 @@ export default class BadgeComponent extends HTMLElement { const conditionalText = predicate ? ' 🥳' : ''; return ( - {count}{conditionalText} + {count}{conditionalText} ); } } From 4c63f7658e158f5a05b4c99008af056beb93dccf Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 8 Jan 2024 21:19:13 -0500 Subject: [PATCH 4/7] update docs --- docs/pages/docs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/docs.md b/docs/pages/docs.md index 8e7a9c2..ed81fdb 100644 --- a/docs/pages/docs.md +++ b/docs/pages/docs.md @@ -331,10 +331,10 @@ export default class Counter extends HTMLElement { render() { const { count } = this; + // note that {count} has to be wrapped in its own HTML tag return (
- You have clicked {count} times
From 0782c3c64a251a62d0584b93b0c8e1c55b9e0a6d Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 8 Jan 2024 21:21:35 -0500 Subject: [PATCH 5/7] fix linting --- test/cases/jsx/jsx.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cases/jsx/jsx.spec.js b/test/cases/jsx/jsx.spec.js index f22686e..834a374 100644 --- a/test/cases/jsx/jsx.spec.js +++ b/test/cases/jsx/jsx.spec.js @@ -70,7 +70,8 @@ describe('Run WCC For ', function() { it('should handle an assignment expression with implicit reactivity using this.render', () => { const element = Array.from(buttons).find(button => button.getAttribute('id') === 'evt-assignment'); - expect(element.getAttribute('onclick')).to.be.equal('this.parentElement.parentElement.count-=1; this.parentElement.parentElement.setAttribute(\'count\', this.parentElement.parentElement.count);'); + expect(element.getAttribute('onclick')) + .to.be.equal('this.parentElement.parentElement.count-=1; this.parentElement.parentElement.setAttribute(\'count\', this.parentElement.parentElement.count);'); }); }); From ab80fb66d547568b775b14b997f72aac79d8f307 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Mon, 8 Jan 2024 21:36:21 -0500 Subject: [PATCH 6/7] update fixture --- .../fixtures/attribute-changed-callback.txt | 112 ++++++++++++------ 1 file changed, 74 insertions(+), 38 deletions(-) diff --git a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt index 87fb721..6dd712b 100644 --- a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt +++ b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt @@ -1,43 +1,79 @@ -attributeChangedCallback(name, oldValue, newValue) { - function getValue(value) { - return value.charAt(0) === '{' || value.charAt(0) === '[' ? JSON.parse(value) : !isNaN(value) ? parseInt(value, 10) : value === 'true' || value === 'false' ? value === 'true' ? true : false : value; - } - if (newValue !== oldValue) { - switch (name) { - case 'count': - this.count = getValue(newValue); - break; +export const inferredObservability = true; +export default class Counter extends HTMLElement { + static get observedAttributes() { + return ['count']; + } + attributeChangedCallback(name, oldValue, newValue) { + console.debug('???attributeChangedCallback', { name }); + console.debug('???attributeChangedCallback', { oldValue }); + console.debug('???attributeChangedCallback', { newValue }); + function getValue(value) { + return value.charAt(0) === '{' || value.charAt(0) === '[' ? JSON.parse(value) : !isNaN(value) ? parseInt(value, 10) : value === 'true' || value === 'false' ? value === 'true' ? true : false : value; } - this.update(name, oldValue, newValue); - } -} -update(name, oldValue, newValue) { - console.debug('Update tracking against....', this.constructor.observedAttributes); - console.debug('Updating', name); - console.debug('Swap old', oldValue); - console.debug('For new', newValue); - console.debug('this[name]', this[name]); - const attr = `data-wcc-${ name }`; - const selector = `[${ attr }]`; - console.debug({ attr }); - console.debug({ selector }); - this.querySelectorAll(selector).forEach(el => { - const needle = oldValue || el.getAttribute(attr); - console.debug({ el }); - console.debug({ needle }); - console.debug({ newValue }); - switch (el.getAttribute('data-wcc-ins')) { - case 'text': - el.textContent = el.textContent.replace(needle, newValue); - break; - case 'attr': - if (el.hasAttribute(el.getAttribute(attr))) { - el.setAttribute(el.getAttribute(attr), newValue); + if (newValue !== oldValue) { + switch (name) { + case 'count': + this.count = getValue(newValue); + break; } - break; + this.update(name, oldValue, newValue); } - }); - if (['count'].includes(name)) { } - console.debug('****************************'); + update(name, oldValue, newValue) { + console.debug('Update tracking against....', this.constructor.observedAttributes); + console.debug('Updating', name); + console.debug('Swap old', oldValue); + console.debug('For new', newValue); + console.debug('this[name]', this[name]); + const attr = `data-wcc-${ name }`; + const selector = `[${ attr }]`; + console.debug({ attr }); + console.debug({ selector }); + this.querySelectorAll(selector).forEach(el => { + const needle = oldValue || el.getAttribute(attr); + console.debug({ el }); + console.debug({ needle }); + console.debug({ newValue }); + switch (el.getAttribute('data-wcc-ins')) { + case 'text': + el.textContent = el.textContent.replace(needle, newValue); + break; + case 'attr': + if (el.hasAttribute(el.getAttribute(attr))) { + el.setAttribute(el.getAttribute(attr), newValue); + } + break; + } + }); + if (['count'].includes(name)) { + } + console.debug('****************************'); + } + constructor() { + super(); + this.count = 0; + } + increment() { + this.count += 1; + this.render(); + } + decrement() { + this.count -= 1; + this.render(); + } + connectedCallback() { + this.render(); + } + render() { + const {count} = this; + this.innerHTML = `
+ +

Counter JSX

+ + + You have clicked ${ count } times + + +
`; + } } \ No newline at end of file From 7e6054a0b7b496acb67d4897e391122a040e3195 Mon Sep 17 00:00:00 2001 From: Owen Buckley Date: Tue, 9 Jan 2024 18:56:11 -0500 Subject: [PATCH 7/7] add attribute to inferredObservablity test case --- .../fixtures/attribute-changed-callback.txt | 45 ++++--------------- .../fixtures/get-observed-attributes.txt | 2 +- .../src/counter.jsx | 5 ++- 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt index 6dd712b..6fa9d6a 100644 --- a/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt +++ b/test/cases/jsx-inferred-observability/fixtures/attribute-changed-callback.txt @@ -1,9 +1,4 @@ -export const inferredObservability = true; -export default class Counter extends HTMLElement { - static get observedAttributes() { - return ['count']; - } - attributeChangedCallback(name, oldValue, newValue) { +attributeChangedCallback(name, oldValue, newValue) { console.debug('???attributeChangedCallback', { name }); console.debug('???attributeChangedCallback', { oldValue }); console.debug('???attributeChangedCallback', { newValue }); @@ -15,6 +10,9 @@ export default class Counter extends HTMLElement { case 'count': this.count = getValue(newValue); break; + case 'highlight': + this.highlight = getValue(newValue); + break; } this.update(name, oldValue, newValue); } @@ -45,35 +43,10 @@ export default class Counter extends HTMLElement { break; } }); - if (['count'].includes(name)) { + if ([ + 'count', + 'highlight' + ].includes(name)) { } console.debug('****************************'); - } - constructor() { - super(); - this.count = 0; - } - increment() { - this.count += 1; - this.render(); - } - decrement() { - this.count -= 1; - this.render(); - } - connectedCallback() { - this.render(); - } - render() { - const {count} = this; - this.innerHTML = `
- -

Counter JSX

- - - You have clicked ${ count } times - - -
`; - } -} \ No newline at end of file + } \ No newline at end of file diff --git a/test/cases/jsx-inferred-observability/fixtures/get-observed-attributes.txt b/test/cases/jsx-inferred-observability/fixtures/get-observed-attributes.txt index 4421db2..97fc202 100644 --- a/test/cases/jsx-inferred-observability/fixtures/get-observed-attributes.txt +++ b/test/cases/jsx-inferred-observability/fixtures/get-observed-attributes.txt @@ -1,3 +1,3 @@ static get observedAttributes() { - return['count']; + return['count', 'highlight']; } \ No newline at end of file diff --git a/test/cases/jsx-inferred-observability/src/counter.jsx b/test/cases/jsx-inferred-observability/src/counter.jsx index b09ead8..2753eeb 100644 --- a/test/cases/jsx-inferred-observability/src/counter.jsx +++ b/test/cases/jsx-inferred-observability/src/counter.jsx @@ -4,6 +4,7 @@ export default class Counter extends HTMLElement { constructor() { super(); this.count = 0; + this.highlight = 'red'; } increment() { @@ -21,7 +22,7 @@ export default class Counter extends HTMLElement { } render() { - const { count } = this; + const { count, highlight } = this; return (
@@ -29,7 +30,7 @@ export default class Counter extends HTMLElement {

Counter JSX

- You have clicked {count} times + You have clicked {count} times