ProjectEvergreen / ProjectEvergreen/wcc

refactor how definitions and entry points are determined (leverage `default export`)

Open
#117 0 comments 0 reactions 1 assignee View on GitHub

@thescientist13 is already working on this.

Since Mar 29, 2023.

enhancement
Dominant language
JavaScript
Stars
126
Forks
17
PR merge metrics
No merged PRs in 30d

Description

Type of Change

Enhancement / Bug

Summary

As observed in #114 coming over from https://github.com/ProjectEvergreen/greenwood/pull/1079, the current design of WCC in particular around determining metadata for entry points, conveniently wrapping renderToString calls in a wrapping HTML element starts to fall apart when multiple customElement.define statements are in a single file.

So given this input

class Navigation extends HTMLElement {
  connectedCallback() {
    this.innerHTML = `
      <nav>
        <ul>
          <li><a href="/">Home</a></li>
          <li><a href="/about">About</a></li>
          <li><a href="/artists">Artists</a></li>
        <ul>
      </nav>
    `;
  }
}

customElements.define('wcc-navigation', Navigation);

class Header extends HTMLElement {
  connectedCallback() {
    this.innerHTML = `
      <header class="header">>
        <h4>My Personal Blog</h4>
      </header>
    `;
  }
}

export default Header;

This is the current metadata output

{
  html: '\n' +
    '      <wcc-navigation>\n' +
    '        \n' +
    '      <header class="header">&gt;\n' +
    '        <h4>My Personal Blog</h4>\n' +
    '      </header>\n' +
    '    \n' +
    '      </wcc-navigation>\n' +
    '    ',
  metadata: [
    'wcc-navigation': {
      instanceName: 'Navigation',
      moduleURL: [URL],
      source: 'class Navigation extends HTMLElement {\n' +
        '    connectedCallback() {\n' +
        '        this.innerHTML = `\n' +
        '      <nav>\n' +
        '        <ul>\n' +
        '          <li><a href="/">Home</a></li>\n' +
        '          <li><a href="/about">About</a></li>\n' +
        '          <li><a href="/artists">Artists</a></li>\n' +
        '        <ul>\n' +
        '      </nav>\n' +
        '    `;\n' +
        '    }\n' +
        '}\n' +
        "customElements.define('wcc-navigation', Navigation);\n" +
        'class Header extends HTMLElement {\n' +
        '    connectedCallback() {\n' +
        '        this.innerHTML = `\n' +
        '      <header class="header">>\n' +
        '        <h4>My Personal Blog</h4>\n' +
        '      </header>\n' +
        '    `;\n' +
        '    }\n' +
        '}\n' +
        'export default Header;',
      url: [URL],
      isEntry: true
    }
  ]
}

Details

As we can see <wcc-navigation> is considered the entry point, though technically that should belong to original moduleURL passed in, right?

Also, technically isn't the default export always going to be the entry point? This seems to substantiate #23 I suppose?
So it seems like the default export should be the entry point. And from there, if the class definition aligns with a customElements.define call, then that should decided if it is safe to wrap, or at least what the right tag to use is if wrapping is still desired.

Another call out though is that if there is only export default XXXX but no customElements.define how do we tracking this in metadata? Currently we use the tag name as a key, so will need to solve for that too.

Not sure if this is the same thing, or supplemental to https://github.com/ProjectEvergreen/wcc/issues/23 so should review them both as part of doing either these tasks.


This an acknowledged defect, but also requires a bit of rearchitecting I think so flagging as both since I don't the final solution here could be reasonably published as a "patch" fix in the spirit of the definition.

We also want to handle this in our JSX transformation (search for this issue URL)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.