-
Notifications
You must be signed in to change notification settings - Fork 34
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
bind) Make ko.applyBindings work with nodeType 11 (Document Fragment) #98
base: main
Are you sure you want to change the base?
Conversation
The regex covering all valid unicode chars was over 11kb! This replaces it with a simple test; reducing overall library size by ~10%; and, I imagine it's faster than an 11kb regex match.
The 3rd commit should probably be a separate pull request. I'll split it off if you'd like, Brian. @mbest, the nodeType checks are inherited from Knockout 3. Can you think of a reason not to allow applying bindings to shadowRoot / Web Components? Would you like a pull request? It would allow for a clean way of solving this: knockout/knockout#2426 |
This makes sense. I'd expect tests, though. |
In principle this makes sense, and I'm happy to plug in the changes for recognizing type-11 nodes, subject to testing that verifies the behaviour. What's the purpose of the changes to |
The change to Identifier was only to shrink the package size. The Regex is something like 10K characters, but CSP explains why it needs to be that way. That commit is irrelevant to getting applyBindings to work with the shadowRoot. |
@avickers Got it, thanks. I wish there were something better than that Regex, but if there is I haven't seen it. 😞 Otherwise, this is just a verification test away from merging into the next alpha. |
It should be possible to apply bindings to nodeType 11 (Document Fragment).
Among other cases, the shadowRoot of a Shadow DOM / Web Component is a document fragment.
Currently, it is necessary to create a wrapper div as a child of the shadowRoot and parent of the actual template. I can see no reason why binding to the shadowRoot directly should throw an error.