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

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

Open
thescientist13 opened this issue Mar 29, 2023 · 0 comments
Assignees
Labels
bug Something isn't working enhancement Improvement to existing functionality
Milestone

Comments

@thescientist13
Copy link
Member

thescientist13 commented Mar 29, 2023

Type of Change

Enhancement / Bug

Summary

As observed in #114 coming over from ProjectEvergreen/greenwood#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 #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.

@thescientist13 thescientist13 added bug Something isn't working enhancement Improvement to existing functionality labels Mar 29, 2023
@thescientist13 thescientist13 added this to the 1.0 milestone Mar 29, 2023
@thescientist13 thescientist13 changed the title refactor how definitions and entry points are determined refactor how definitions and entry points are determined (leverage default export Mar 29, 2023
@thescientist13 thescientist13 changed the title refactor how definitions and entry points are determined (leverage default export refactor how definitions and entry points are determined (leverage default export) Mar 29, 2023
@thescientist13 thescientist13 self-assigned this Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement Improvement to existing functionality
Projects
Status: 📋 Backlog
Development

No branches or pull requests

1 participant