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

[NO-TICKET] Preact discovery #2368

Closed
wants to merge 18 commits into from
Closed

[NO-TICKET] Preact discovery #2368

wants to merge 18 commits into from

Commits on Jan 31, 2023

  1. Configuration menu
    Copy the full SHA
    c7c05cd View commit details
    Browse the repository at this point in the history
  2. Update examples/react-app to be a direct comparison with the preact…

    … one
    
    This simplifies it by stripping out the Sass stuff. We could possibly add some CSS back in later, but right now I'm focused on the JS part of it.
    
    Some stats:
    - `react-app` produces a bundle weighing 188 KB
    - `preact-app` produces a bundle weighing 76 KB
    pwolfert committed Jan 31, 2023
    Configuration menu
    Copy the full SHA
    07a661c View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    84918dc View commit details
    Browse the repository at this point in the history
  4. Build a preact CDN bundle and use it in a new cdn-preact example

    Note that the additional babel config breaks Storybook, but if we end up going this route, we can update our Storybook config to work with preact
    pwolfert committed Jan 31, 2023
    Configuration menu
    Copy the full SHA
    bdcd14d View commit details
    Browse the repository at this point in the history

Commits on Feb 1, 2023

  1. Web components are kind of working

    If I were doing this for real, I'd make a separate web components bundle so there are no side effects in the npm package. Problem right now is being able to pass `children` to web components. Going to try https://github.com/jahilldev/component-elements/tree/main/packages/preactement#readme next based on [this issue thread](preactjs/preact-custom-element#41)
    pwolfert committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    823f568 View commit details
    Browse the repository at this point in the history
  2. Switched to preactement, but...

    The conversion of the tag contents into the `children` prop still doesn't seem to work, even though the [preactement readme](https://github.com/jahilldev/component-elements/tree/main/packages/preactement) shows an example of it. I'm wondering if it's an issue of support for the latest version of Preact. This library hasn't been updated in nine months. Nope, that can't be it, because their package.json says it wants a peer dependency of `"preact": ">=10"`, which is what we're on.
    pwolfert committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    b709c89 View commit details
    Browse the repository at this point in the history
  3. Tried a different component but same problem

    I've inspected the Alert render function while it's rendering the web component, and `children` does have a value (it's a rendered React/Preact element, so I can't see what's in it), but nothing ends up in the final output, and that content in between the custom element tags is just sitting there below all the rendered html
    pwolfert committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    98f78f9 View commit details
    Browse the repository at this point in the history
  4. Did some more digging...

    to see if I could find out what the contents of the children prop are. I don't think there's anything in it. I stepped through Preact's render function to the part where it reconciles differences with the DOM. My content wasn't there, and I know that doesn't actually prove anything, but I just have a feeling it isn't a problem with anything that happens from the Alert render call down. I do think it's a problem with the value of the `children` prop that is passed to the Alert instance
    pwolfert committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    90bc645 View commit details
    Browse the repository at this point in the history
  5. Passing children to the web components works!!

    Just had to move the script to the end of `<body>` so the full DOM is loaded at the time when the initial value of children is being queried.
    
    See the commit messages for b709c89, 98f78f9, and 90bc645 for details about the problem. In summary, the `children` prop isn't being correctly populated. My most recent experiment in seeing where it goes wrong: print out the value of `this.inner` from [this line of preactement](https://github.com/jahilldev/component-elements/blob/main/packages/preactement/src/define.ts#L134). I actually printed it from [the parseHtml function](https://github.com/jahilldev/component-elements/blob/8c5666ccaad6cbeda73b77a5d0eb430a09071494/packages/preactement/src/parse.ts#L17). The result is a blank printout. Aha! I printed out [the return value of `getDocument`](https://github.com/jahilldev/component-elements/blob/8c5666ccaad6cbeda73b77a5d0eb430a09071494/packages/preactement/src/parse.ts#L17), which was empty `<body></body>` tags, which led me to test moving the bundle script tag out of `<head>` and to the end of `<body>`, and it worked!!!!!
    pwolfert committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    3bdc659 View commit details
    Browse the repository at this point in the history

Commits on Feb 3, 2023

  1. Configure jest to optionally use Preact. Nice mix of pass and fail!

    Some pass and a lot of snapshot tests fail, as to be expected. Will need to go through these and determine if it makes sense to even test snapshots at all for Preact if we expect so many differences in the resultant DOM
    
    ```
    Test Suites: 29 failed, 55 passed, 84 total
    Tests:       66 failed, 636 passed, 702 total
    Snapshots:   56 failed, 122 passed, 178 total
    ```
    pwolfert committed Feb 3, 2023
    Configuration menu
    Copy the full SHA
    c9700ea View commit details
    Browse the repository at this point in the history

Commits on Feb 7, 2023

  1. Use Preact in Storybook

    pwolfert committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    cf6d172 View commit details
    Browse the repository at this point in the history
  2. Failed attempt to mix web components into storybook instance

    Unfortunately I'd need to change the `framework` config for storybook, and you [can't currently mix frameworks](storybookjs/storybook#3889). Going to revert this but wanted a record of the journey
    pwolfert committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    a557886 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    91038bf View commit details
    Browse the repository at this point in the history
  4. Bundle is now just web components

    Until we've gone through each component to make web components versions—and ones that take complex props could be a lot of work—we probably want to have two different bundles. My next commit, therefore, will be to build two different bundles
    pwolfert committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    c2600f5 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    0aa5c4d View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    69819bb View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    8864d3e View commit details
    Browse the repository at this point in the history
  8. Add button to explore interactivity and non-string prop types

    Unfortunately we have no way as it is now to pass boolean props. I tried setting `analytics="true"`, but the Preact component receives that as a string. We will probably need a layer of abstracting components that translate between attributes and props in a more intentional way. There are also components like MonthPicker with array props that will need a lot of work in this area
    pwolfert committed Feb 7, 2023
    Configuration menu
    Copy the full SHA
    3e91947 View commit details
    Browse the repository at this point in the history