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

bug/issue 128 fix shadow root rendering for JSX #129

Merged
merged 11 commits into from
Jan 6, 2024

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented Jan 3, 2024

Related Issue

resolves #128

Summary of Changes

  1. Wrap JSX that renders into a shadow DOM with a <template> tag
  2. Fix issues with inferredObservability interop
  3. Added a test case (there was never one to begin with 😱 )
  4. All sandbox demos are working as expected now 🎉
  5. Updated documentation, examples, and caveats for JSX support

See thescientist13/greenwood-htmx#10 for a working example

TODO

  1. JSX + Shadow DOM testing - see browser warning about "declarative Shadow DOM has not been enabled by includeShadowRoots" #130
    • so yeah, the current implementation is not going to work, if use shadowRoot.innerHTML we're still going to keep getting runtime warnings
      Screenshot 2024-01-03 at 6 53 46 PM
  2. Greenwood regression testing
  3. more test cases, like for inferredObservability
  4. documentation
  5. reuse HTMLTemplateElement (e.g. DOM shim) instead of manually wrapping - N / A
  6. refactor DSD render function implementation to avoid innerHTML usage (nice to have, otherwise could be a good first issue) - made its own issue, but of lower priority if we can get Fine Grained Inferred Observability for JSX #108 working (since renders would only happen once) - optimize how initial HTML is set on initial render for JSX usage #138

@thescientist13 thescientist13 added bug Something isn't working documentation Improvements or additions to documentation labels Jan 3, 2024
@thescientist13 thescientist13 self-assigned this Jan 3, 2024
@thescientist13 thescientist13 force-pushed the bug/issue-128-dsd-and-jsx-compat branch from 99478ea to 64b8d99 Compare January 6, 2024 00:27
@thescientist13 thescientist13 marked this pull request as ready for review January 6, 2024 02:02
@@ -0,0 +1,27 @@
export default class HeadingComponent extends HTMLElement {
sayHello() {
alert('hello world!');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use this.greeting instead of hardcode world

@thescientist13 thescientist13 force-pushed the bug/issue-128-dsd-and-jsx-compat branch from 0ad5490 to 91b9282 Compare January 6, 2024 18:37
@thescientist13 thescientist13 merged commit 6abeca4 into master Jan 6, 2024
12 checks passed
@thescientist13 thescientist13 deleted the bug/issue-128-dsd-and-jsx-compat branch January 6, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
1 participant