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 69 jsx transformations #73

Merged
merged 41 commits into from
Aug 13, 2022

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented Jul 9, 2022

Related Issue

#69 / #83

To run locally, clone this branch, run npm ci, and then run npm run sandbox:serve

Summary of Changes

  1. Basic demonstration and proof of concept for supporting JSX based transformations through a render function for custom elements.
  2. At the moment, using NodeJS custom loaders (--experimental-loader) to support being able to import a JSX file
  3. Split testing to cover Node versions with and without support for --expiremental-loader
class Counter extends HTMLElement {
  constructor() {
    super();
    this.count = 0;
  }

  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
    }

    this.render();
  }

  render() {
    const { count } = this;

    return (
      <div>
        <h3>Counter</h3>
        <button onclick={this.count -= 1}> - </button>
        <span>You have clicked {count} times</span>
        <button onclick={this.count += 1}> + </button>
      </div>
    );
  }
}

customElements.define('wcc-counter', Counter);

Would render down to something like

class Counter extends HTMLElement {
  constructor() {
    super();
    this.count = 0;
  }

  connectedCallback() {
    if (!this.shadowRoot) {
        this.attachShadow({ mode: 'open' });
    }
    this.render();
  }

  render() {
    const {count} = this;
    
    this.shadowRoot.innerHTML = `
      <div>
        <h3>Counter</h3>
        <button onclick="this.parentElement.parentNode.host.count-=1; this.parentElement.parentNode.host.render();"> + </button>
        <span>You have clicked ${ count } times</span>
        <button onclick="this.parentElement.parentNode.host.count+=1; this.parentElement.parentNode.host.render();"> - </button>
      </div>
    `;
  }
}

customElements.define('wcc-counter', Counter);

Initial Feature Set

  • Can use class for styles (not className)
  • Can use onxxx for events (not onXyz)
  • Event Handling with auto this resolution
  • Automatic Shadow DOM detection
  • Naive reactivity on assignment expressions

TODO

  1. Validate returning serialized HTML from JSX (e.g. fully operational renderToString)
  2. Be able to import a JSX file, but would need to leverage experimental loaders for this, otherwise Node.js will blow up on .jsx files, related to support resource plugin based transformations for standard module formats (ex: import JSON, CSS) for SSR greenwood#878
    (node:84287) UnhandledPromiseRejectionWarning: TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".jsx" for /Users/owenbuckley/Workspace/project-evergreen/repos/wcc/test/cases/jsx/src/counter.jsx
  3. Validate recursive rendering
  4. Create a TODO App example (hosted on Vercel) with styles and spin off into separate repo - Migrate Greenwood and Lit 2 todo-app#28
  5. Flesh out test cases for JSX and get --experimental-loader support for testing task
  6. Windows support / get build passing
  7. Evaluate tagged template literal / template example results based implementation for reference
    • Not sure if it would just be easier to use JSX, since it is already a specification and there is an AST for it? Instead of rolling my own?
    • Not sure if the tagged template version will come with enough "fidelity" for AST parsing, unless again, I come up with my own spec...?
    • Also, JSX already has syntax highlighting supports, and a bit more ergonomic of an authoring experience - https://facebook.github.io/jsx/#sec-why-not-template-literals
  8. Audit JSX use cases and specs to avoid missing any obvious gaps, track others as new issues - JSX Syntax Enhancements and Features #84
  9. Need to make this experimental / opt-in, or will the combination of a render function + ,jsx file be enough?
    • If no .jsx file detection support, then it should definitely be opt-in
    • will be based on --experimental-loader
  10. Some kind of documentation / clean up demo code
    • minimum node version of16.x, otherwise 14.x
    • --experimental-loader API
    • examples for frameworks
  11. Cherry pick a few JSX enhancements to track from the discussion as issues (groom TODOs from the code)
    • form submit
    • observability / reactivity
    • this expressions
  12. Clean up demo code and sandbox code
  13. Round up remaining TODOs
  14. Refactor
    • Organize into own file
    • Refactor out Shadow DOM detection to use an AST?
  15. (stretch) hydration support

Questions and Features + Syntax Enhancements

  1. Noticed race conditions issueshttps://github.com/Feature/issue 69 jsx transformations #73/commits/9b95b61b50627b2116fd360c5695328a2c2cde10#diff-7c4263d59f201b16b190a22ad58b36a6975060d95031aeba98f7f0ab9e490c1fR152 with async visitor functions, will need to not make them async - async AST visitor functions causing race conditions #75
  2. How to best give JSX contents / metadata to consumers (URLs, compiled source, etc). What to do about .jsx extensions in source files? - JSX Syntax Enhancements and Features #84
  3. Reactivity / auto render for something like onclick={() => this.count += 1} - JSX Syntax Enhancements and Features #84
    • Feature/issue 69 jsx transformations #73 (comment)
    • how to automatically render when just touching state? e.g. calls to an update / patch method vs this.render()?
    • can we leverage setters and getters (attributes vs properties) so addTodo doesn't also have to call this.render()?
    • is this fine grained reactivity?
  4. Support loops in JSX - JSX Syntax Enhancements and Features #84
      <ol>
         ${repeat(todos, (todo) => Date.now() + todo.id, (todo) => html`
           <li>
             <x-todo-list-item
               todo=${todo}
             ></x-todo-list-item>
           </li>
         `)}
       </ol>
  5. Support expression you have {this.todos.length} TODOs left to complete. As it is a literal evaluation, it will throw an error in NodeJS when rendering - ReferenceError: __this__ is not defined - JSX Syntax Enhancements and Features #84
  6. CSS (Light and Shadow) - From experience, JSX does not support <style> tag in JSX (need to link to a source) and so would like to align with CSS solutions and support #33 to figure out a way to get first party CSS support into custom elements in an SSR and platform compatible way. - JSX Syntax Enhancements and Features #84
  7. Support destructured this references - JSX Syntax Enhancements and Features #84
    render() {
      const { id  } = this.todo;
    
      return (
         <button onclick={() => this.dispatchDeleteTodoEvent(id)}></button>
      );
    }
  8. Support checked like attributes - JSX Syntax Enhancements and Features #84
    render() {
      const { completed, task } = this.todo;
      const checked = completed ? 'checked' : '';
    
      // JSX doesn't like {checked}
      // and even just setting checked will automatically set the checkbox to true according to the spec (e.g. it must be all or nothing
      return (
        <input class="complete-todo" type="checkbox" checked onchange={this.dispatchCompleteTodoEvent}/>
      );
    }
  9. Better way to handle init render - Feature/issue 69 jsx transformations #73 (comment)
  10. Class name as component name </Header> - JSX Syntax Enhancements and Features #84
  11. Other options for ESM + .jsx files - Feature/issue 69 jsx transformations #73 (comment)
  12. Declarative Shadow DOM + reactivity - Feature/issue 69 jsx transformations #73 (comment)
  13. Is this the genesis of HydrateElement or something else entirely? - HydrateElement base class prototyping #29
  14. Start of a Plugin / Transform system (maybe too soon so save for 2.0. make a discussion?) - Plugin / Adapter Architecture #90
  15. Fragment <></> - JSX Syntax Enhancements and Features #84
  16. Reach out to this Reddit thread?

@thescientist13 thescientist13 added the feature New feature or request label Jul 9, 2022
@thescientist13 thescientist13 added this to the 1.0 milestone Jul 9, 2022
@thescientist13 thescientist13 self-assigned this Jul 9, 2022
@thescientist13
Copy link
Member Author

thescientist13 commented Jul 15, 2022

So this is interesting. Realized that although we were rendering JSX into a Shadow Root, I was not actually return the content into a declarative shadow root. So in the control group for example I had this

# before
class CounterControlShadow extends HTMLElement {
  constructor() {
    super();
    this.count = 0;
  }

  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
    }
    this.render();
  }

  increment() {
    this.count += 1;
    this.render();
  }

  decrement() {
    this.count -= 1;
    this.render();
  }

  render() {
    const { count } = this;

    this.shadowRoot.innerHTML = `
      <div>
        <h3>CounterControlShadow</h3>
        <button id="dec" onclick="this.parentElement.parentNode.host.decrement()"> - </button>
        <span>You have clicked <span id="count">${count}</span> times</span>
        <button id="inc" onclick="this.parentElement.parentNode.host.increment();"> + </button>
      </div>
    `;
  }
}

customElements.define('wcc-counter-control-shadow', CounterControlShadow);

So when I "fixed" the render function to this

  render() {
    const { count } = this;

    this.shadowRoot.innerHTML = `
      <template shadowroot="open">
        <div>
          <h3>CounterControlShadow</h3>
          <button id="dec" onclick="this.parentElement.parentNode.host.decrement()"> - </button>
          <span>You have clicked <span id="count">${count}</span> times</span>
          <button id="inc" onclick="this.parentElement.parentNode.host.increment();"> + </button>
        </div>
      </template>
    `;
  }

And then was getting this this error in the browser on load
Screen Shot 2022-07-14 at 20 43 46

So this could be an issue with continuously redeclaring the <template> tag when setting innerHTML? not sure if this would get fixed with more explicit / less naive reactivity model, but found some links that might be relevant - https://stackoverflow.com/questions/67932949/html-template-shadow-dom-not-rendering-within-handlebars-template .

@thescientist13
Copy link
Member Author

thescientist13 commented Jul 15, 2022

On the decision to use --experimental-loader to support being able to use ESM for .jsx files given as the name implies, is experimental, obviously requires an entire consuming application to have to invoke itself with that flag to use it, which is fine in terms of being an opt-in API, but obviously a very heavy handed one, as opposed to a config flag.

node --expiremental-loader /path/to/loader.js my-script.js

While I do like how it is a first party API for Node, I believe its the only way this would be possible with JSX because even if we supported .js files, the fact that there is "exposed" HTML will cause the loader to fail still.

% yarn sandbox
yarn run v1.22.17
$ node --trace-warnings --experimental-loader ./src/jsx-loader.js ./sandbox.js
(node:21030) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
    at emitExperimentalWarning (node:internal/util:224:11)
    at initializeLoader (node:internal/process/esm_loader:59:3)
    at loadESM (node:internal/process/esm_loader:87:11)
    at runMainESM (node:internal/modules/run_main:51:21)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:74:5)
    at node:internal/main/run_main_module:17:47
load file:///Users/owenbuckley/Workspace/project-evergreen/wcc/test/cases/jsx/src/counter.jsx
file:///Users/owenbuckley/Workspace/project-evergreen/wcc/test/cases/jsx/src/counter.jsx:25
      <div>
      ^

SyntaxError: Unexpected token '<'
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:115:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:289:14)
    at async link (node:internal/modules/esm/module_job:70:21)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Not sure what other options there would be if ESM is to be a first class citizen? Part of this reason is to support .jsx files, but also because we want to be able to import these files for their export statements, like a getData function

export default class MyElement extends HTMLElement {
  ...
}

export async function getData() {
   return { /* ... */ }
}
async function initializeCustomElement(elementURL, tagName, attrs = []) {
  await registerDependencies(elementURL);

  // https://github.com/ProjectEvergreen/wcc/pull/67/files#r902061804
  const { pathname } = elementURL;
  const element = tagName
    ? customElements.get(tagName)
    : (await import(pathname)).default;
  const dataLoader = (await import(pathname)).getData;
  const data = dataLoader ? await dataLoader() : {};
  const elementInstance = new element(data); // eslint-disable-line new-cap

  attrs.forEach((attr) => {
    elementInstance.setAttribute(attr.name, attr.value);

    if (attr.name === 'hydrate') {
      definitions[tagName].hydrate = attr.value;
    }
  });

  await elementInstance.connectedCallback();

  return elementInstance;
}

Maybe there is another way to do that?

@thescientist13
Copy link
Member Author

Not sure if it is a JSX thing or not, but this seems to completely break the compiler when there is no initial render call made to set innerHTML.

export default class App extends HTMLElement {
  constructor() {
    super();
  }

  render() {
    return (
      <div>
        <h1>TODO App</h1>
      </div>
    );
  }
}

customElements.define('todo-app', App);

Maybe an add-on case for #4 ?

@thescientist13
Copy link
Member Author

thescientist13 commented Jul 21, 2022

Seeing some patterns emerging for implicit reactivity based on parsing the JSX. Take this example from the TODO List App.

class TodoListItem extends HTMLElement {

  constructor() {
    super();

    this.todo = {};
  }

  static get observedAttributes () {
    return ['todo'];
  }

  attributeChangedCallback(name, oldValue, newValue) {
    if (newValue !== oldValue) {
      if (name === 'todo') {
        this.todo = JSON.parse(newValue);
      }

      this.render();
    }
  }

  render() {
    const { completed, task } = this.todo;
    const completionStatus = completed ? '✅' : '⛔';
    
    return (
      <span>
        {task}

        <span>{completionStatus}</span>

        <button onclick={() => this.dispatchDeleteTodoEvent(this.todo.id)}></button>
            
      </span>
    );
  }
}

customElements.define('todo-list-item', TodoListItem);

I'm sure I could probably autowire observedAttributes based on what's in the render function and / or the constructor. changedAttributeCallback might be a bit harder to accurately predict but maybe worth a shot to at least explore it?

This advantage is users not having to manage call render every time an action happens.

I suppose the risk is what about an escape hatch? If a user needs to define their own, how do we ensure JSX doesn't clobber over it.

@thescientist13 thescientist13 force-pushed the feature/issue-69-jsx-transformations branch from c6f4d94 to 6099044 Compare July 27, 2022 12:29
@thescientist13 thescientist13 force-pushed the feature/issue-69-jsx-transformations branch from f469155 to 647ebf4 Compare July 29, 2022 02:01
@thescientist13 thescientist13 linked an issue Aug 3, 2022 that may be closed by this pull request
@thescientist13 thescientist13 removed this from the 1.0 milestone Aug 3, 2022
@thescientist13 thescientist13 added documentation Improvements or additions to documentation and removed v0.6.0 labels Aug 3, 2022
@thescientist13 thescientist13 marked this pull request as ready for review August 9, 2022 02:03
@thescientist13 thescientist13 removed their assignment Aug 10, 2022
@thescientist13 thescientist13 merged commit 862d054 into master Aug 13, 2022
@thescientist13 thescientist13 deleted the feature/issue-69-jsx-transformations branch August 13, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation expirement feature New feature or request JSX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile JSX to custom element (proof of concept)
1 participant