Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/issue 108 fine grained observability #110

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions docs/pages/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -331,6 +331,7 @@ export default class Counter extends HTMLElement {
render() {
const { count } = this;

// note that {count} has to be wrapped in its own HTML tag
return (
<div>
<button onclick={this.count -= 1}> -</button>
Expand All @@ -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).
2 changes: 1 addition & 1 deletion sandbox/components/counter-dsd.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down
85 changes: 80 additions & 5 deletions src/jsx-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

const jsxRegex = /\.(jsx)$/;

// TODO same hack as definitions

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO same hack as definitions'

Check warning on line 12 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO same hack as definitions'
// https://github.com/ProjectEvergreen/wcc/discussions/74
let string;

// TODO move to a util

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO move to a util'

Check warning on line 16 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO move to a util'
// https://github.com/ProjectEvergreen/wcc/discussions/74
function getParse(html) {
return html.indexOf('<html>') >= 0 || html.indexOf('<body>') >= 0 || html.indexOf('<head>') >= 0
Expand Down Expand Up @@ -101,7 +101,7 @@
}

// onclick={() => this.deleteUser(user.id)}
// TODO onclick={(e) => { this.deleteUser(user.id) }}

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (20)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO onclick={(e) => {...'

Check warning on line 104 in src/jsx-loader.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO onclick={(e) => {...'
// TODO onclick={(e) => { this.deleteUser(user.id) && this.logAction(user.id) }}
// https://github.com/ProjectEvergreen/wcc/issues/88
if (expression.type === 'ArrowFunctionExpression') {
Expand All @@ -117,7 +117,8 @@
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});"`;
}
}
}
Expand Down Expand Up @@ -145,6 +146,9 @@
}
}
}

// TODO make sure this only applies to `this` references!
string += ` data-wcc-${expression.name}="${name}" data-wcc-ins="attr"`;
}
} else {
// xxx >
Expand All @@ -171,6 +175,11 @@

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;
Expand Down Expand Up @@ -217,10 +226,16 @@
// 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);
}
});
}
Expand Down Expand Up @@ -324,15 +339,40 @@
}

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) {
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)
Expand All @@ -344,19 +384,54 @@
}
if (newValue !== oldValue) {
switch(name) {
${observedAttributes.map((attr) => {
${trackingAttrs.map((attr) => {
return `
case '${attr}':
this.${attr} = getValue(newValue);
break;
`;
}).join('\n')}
}
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;
}
})

this.render();
if ([${[...trackingAttrs].map(attr => `'${attr}'`).join()}].includes(name)) {
${derivedSetters}
}
console.debug('****************************');
}

${derivedGetters}

${newModuleContents.slice(insertPoint)}
`;
/* eslint-enable indent */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,52 @@
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();
}
}
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;
}
if (newValue !== oldValue) {
switch (name) {
case 'count':
this.count = getValue(newValue);
break;
case 'highlight':
this.highlight = 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',
'highlight'
].includes(name)) {
}
console.debug('****************************');
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
static get observedAttributes() {
return['count'];
return['count', 'highlight'];
}
5 changes: 3 additions & 2 deletions test/cases/jsx-inferred-observability/src/counter.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export default class Counter extends HTMLElement {
constructor() {
super();
this.count = 0;
this.highlight = 'red';
}

increment() {
Expand All @@ -21,15 +22,15 @@ export default class Counter extends HTMLElement {
}

render() {
const { count } = this;
const { count, highlight } = this;

return (
<div>
<wcc-badge count={count}></wcc-badge>
<h3 data-test="hello123">Counter JSX</h3>
<button id="evt-this" onclick={this.decrement}> - (function reference)</button>
<button id="evt-assignment" onclick={this.count -= 1}> - (inline state update)</button>
<span>You have clicked <span class="red" id="expression">{count}</span> times</span>
<span>You have clicked <span class={highlight} id="expression">{count}</span> times</span>
<button onclick={this.count += 1}> + (inline state update)</button>
<button onclick={this.increment}> + (function reference)</button>
</div>
Expand Down
2 changes: 1 addition & 1 deletion test/cases/jsx-shadow-dom/src/heading.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class HeadingComponent extends HTMLElement {

return (
<div>
<h1>Hello, {greeting}!</h1>
<h1>Hello, <span>{greeting}</span>!</h1>
<button onclick={this.sayHello}>Get a greeting!</button>
</div>
);
Expand Down
4 changes: 3 additions & 1 deletion test/cases/jsx/jsx.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -69,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.render();');
expect(element.getAttribute('onclick'))
.to.be.equal('this.parentElement.parentElement.count-=1; this.parentElement.parentElement.setAttribute(\'count\', this.parentElement.parentElement.count);');
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/cases/jsx/src/badge.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default class BadgeComponent extends HTMLElement {
const conditionalText = predicate ? ' 🥳' : '';

return (
<span class={conditionalClass}>{count}{conditionalText}</span>
<span class={conditionalClass}><span>{count}</span>{conditionalText}</span>
);
}
}
Expand Down
Loading