-
Notifications
You must be signed in to change notification settings - Fork 79
Implement custom elements interface for Crank components #307
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
base: main
Are you sure you want to change the base?
Conversation
• Add createCustomElementClass() function to convert Crank components into CustomElement classes • Attribute changes are batched using microtasks for optimal performance • Components receive ref callback to extend custom element with methods/properties • Full shadow DOM support with automatic slot projection to individual props • EventTarget bridging allows component events to bubble to custom element • Custom event properties (oncustomevent) work beyond DOM spec capabilities • Update DOMRenderer to support rendering to ShadowRoot • Add comprehensive test suite with 8 tests covering all functionality • Include working examples demonstrating counter and card components 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
brainkim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
examples/custom-elements.html
Outdated
| @@ -0,0 +1,145 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brainkim1 This should be a JavaScript file. See examples for examples of examples.
examples/custom-elements.html
Outdated
| import { createCustomElementClass } from "../dist/custom-elements.js"; | ||
|
|
||
| // Define a Counter component | ||
| function Counter({ count, ref }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example could be more interesting. How about we revive marquee and blink as custom components?
src/custom-elements.ts
Outdated
| this._scheduleUpdate(); | ||
| } | ||
|
|
||
| private _parseSlots(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the convention of hidden Symbol methods for private methods. This is the public element interface.
src/custom-elements.ts
Outdated
| renderer: customRenderer = renderer | ||
| } = options; | ||
|
|
||
| class CrankCustomElement extends HTMLElement { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have the name of the custom element in the generated class. If this is possible. So Constructor.name would be the component name, somehow.
src/custom-elements.ts
Outdated
| } = options; | ||
|
|
||
| class CrankCustomElement extends HTMLElement { | ||
| private _props: Partial<TProps> = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use declare [_props]: Partial;
No initializers, all properties should be defined in the constructor.
| this._props[name as keyof TProps] = newValue as any; | ||
| this._scheduleUpdate(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t use the “upgrade” part of the CustomElement lifecycle, maybe that can be when we call the callback which the ref gives us (I know kinda mind-bending, but something about the ref callback being called with a callback during upgrade makes me feel some ways.
| } else if ((root as Node).nodeType !== Node.ELEMENT_NODE) { | ||
| } else if ( | ||
| (root as Node).nodeType !== Node.ELEMENT_NODE && | ||
| (root as Node).nodeType !== Node.DOCUMENT_FRAGMENT_NODE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Are there other valid roots we should consider?
- Convert HTML example to JavaScript file with marquee/blink components - Use Symbol methods for private class methods - Add component name to generated custom element class - Use declare syntax without initializers for better TypeScript style - Fix hasOwnProperty linting error 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
brainkim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More thoughts
| })); | ||
|
|
||
| this.after((el) => { | ||
| const container = el.querySelector(".marquee-container"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do host styles.
examples/custom-elements.js
Outdated
|
|
||
| ref?.((element) => ({ | ||
| start() { | ||
| this.play(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object is going to be assigned to the element why not use element.play(). The this of the Crank component are separate but linked, and the context does not have this.play(), this.pause() on it.
examples/custom-elements.js
Outdated
|
|
||
| // Create custom element classes | ||
| const MarqueeElement = createCustomElementClass(Marquee, { | ||
| observedAttributes: ["speed", "playing"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know this could be generated based on the types of the parameters, but doing it explicity is nice I guess.
examples/custom-elements.js
Outdated
| contentWidth = content.offsetWidth; | ||
| }); | ||
|
|
||
| const interval = setInterval(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s use raf, and let’s use the refresh() callback pattern introduced in 0.7
examples/custom-elements.js
Outdated
| }, 16); // ~60fps | ||
|
|
||
| try { | ||
| for ({speed, children} of this) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the type of speed just string?
examples/custom-elements.js
Outdated
| // Bounce effect | ||
| if (position > containerWidth) { | ||
| position = -contentWidth; | ||
| this.dispatchEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The events should likely be dispatched in an after to prevent re-entrancy issues (rendering while rendering)
examples/custom-elements.js
Outdated
| }); | ||
|
|
||
| // Register custom elements | ||
| customElements.define("x-marquee", MarqueeElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rip-marquee, rip-blink. Put blink first because it’s an easier component to write.
examples/custom-elements.js
Outdated
| </x-marquee> | ||
|
|
||
| <div class="controls"> | ||
| <button onclick="document.querySelectorAll('x-marquee').forEach(m => m.play())"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crank does not allow arbitrary code as string in event callbacks. You might have to write a component which consumes to marquee and has state with a generator.
examples/custom-elements.js
Outdated
| <button onclick="document.querySelectorAll('x-marquee').forEach(m => m.play())"> | ||
| Play All | ||
| </button> | ||
| <button onclick="document.querySelectorAll('x-marquee').forEach(m => m.pause())"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
examples/custom-elements.js
Outdated
|
|
||
| eventMarquee.onmarqueebounce = (e) => { | ||
| // eslint-disable-next-line no-console | ||
| console.log("Marquee bounced!", e.detail); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples should avoid having console.log calls. Rather you should add to the example in some way that is visible.
- Fix ref object methods to use element parameter instead of this - Use requestAnimationFrame instead of setInterval for smoother animation - Implement refresh() callback pattern from 0.7 for state updates - Handle attribute values as strings with proper parsing - Dispatch events in this.after() to prevent re-entrancy issues - Rename elements to rip-blink and rip-marquee (RIP = Rest In Peace) - Reorganize with Blink component first as it's simpler - Add host styles for custom elements in demo 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Summary
This PR implements a comprehensive custom elements interface that allows converting Crank components into Web Components with seamless integration and performance optimizations.
Key Features
•
createCustomElementClass()- Converts Crank components to CustomElement classes• Microtask Batching - Attribute changes are automatically batched for optimal performance
• Ref API - Components can extend custom elements with methods/properties via callback
• Shadow DOM Support - Full support for open/closed shadow DOM with slot projection
• EventTarget Bridging - Component events properly bubble to custom elements
• Custom Event Properties - Support for
oncustomeventproperties beyond DOM specImplementation Highlights
• Updated DOMRenderer to support rendering to ShadowRoot (fixes oversight)
• Slot projection parses light DOM children by slot attribute into individual props
• EventTarget delegation bridges component context to custom element seamlessly
• Custom event properties automatically called on dispatchEvent for enhanced DX
• Comprehensive test coverage with 8 tests ensuring reliability
Usage Example
Test Coverage
All 501 tests pass including 8 comprehensive custom elements tests covering:
• Basic element creation and rendering
• Attribute reactivity
• Ref API functionality
• Shadow DOM with slots
• Event bridging and custom properties
🤖 Generated with Claude Code