-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix(jsx): correct behaviour #690
Conversation
Run & review this pull request in StackBlitz Codeflow. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
6271316
to
18dfd98
Compare
- `className` (for compatibility with React libraries like [`stylerun`](https://github.com/artalar/stylerun)) | ||
- `innerHTML` | ||
- `innerText` |
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 avoid innerHTML
and innerText
defaults as they are very rarely used in regular application usage. className
bindings may be an extra thing here too; I will think about it.
onConnect={(ctx) => console.log('component connected')} | ||
onDisconnect={(ctx) => console.log('component disconnected')} |
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.
onDisconnect
is an extra thing. Let's stick with onConnect(() => () => cleanup())
. Also, since this property is specific to the reatom/jsx runtime, let's mark it with a special prefix like $connect
.
However, the hook bindings to the returned element are very weird (poor semantics), and we should find another way.
A possible compromise solution could be the $ref
property, similar to React, which accepts a callback with ctx
and the connected element
as arguments. The callback could return the cleanup function.
@@ -38,7 +38,8 @@ | |||
"@reatom/core": ">=3.5.0", | |||
"@reatom/lens": "^3.6.2", | |||
"@reatom/utils": "^3.5.0", | |||
"csstype": "^3.1.2" | |||
"csstype": "^3.1.2", | |||
"linkedom": "^0.16.4" |
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.
Move it to devDependencies
@@ -1,10 +1,152 @@ | |||
import { test } from 'uvu' | |||
import * as assert from 'uvu/assert' | |||
import { parseHTML } from 'linkedom' |
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.
I'm glad that you set up these tests. Good job! 👍
No description provided.