-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
refactor!: rewrite #46
base: master
Are you sure you want to change the base?
Conversation
I can add tests for Solid if you'd like :) That said, I know that the setCurrentInstance was a blocker previously. I generally agree we should wait until that PR merges and releases before this moves forward |
Yeah, feel free to take it. I was thinking the same about |
Generally LGTM, I think you could take the lead and merge it when you think it's ready. To be careful, we could do a few beta release before the stable one. And also it would be great to have a migration guide in readme. |
Now, export const App = defineComponent(() => {
const counter = ref(0)
return computed(() => (
<div>
<p>Count: {counter.value}</p>
<button onClick={() => (counter.value += 1)}>Increment</button>
</div>
))
}) |
I like the new interface, and well done! I am fine with Otherwise, feel free to merge when you think it's ready. You can ping me when there is any action needs on my side. Thank you! |
Is there some body can merge this PR ? |
Is there any work left to do on this? Would love to help out if needed. |
export const App = () => {
const counter = ref(0)
return (
<div>
<p>Count: {counter.value}</p>
<button onClick={() => (counter.value += 1)}>Increment</button>
</div>
)
} Will be compiled to: export const App = defineComponent(() => {
const counter = ref(0)
return computed(() => (
<div>
<p>Count: {counter.value}</p>
<button onClick={() => (counter.value += 1)}>Increment</button>
</div>
))
}) Babel plugin shouldn't be part of this package.
I will not be able to work on all these stuff for the foreseeable future. For the time being I'm not interested in working on experimental stuff. If you want to push it forward feel free to make a PR for the |
Would be possible to omit the |
I believe that a valuable action right now would be a small-scale refactoring to make the 0.x version compatible with Vue 3.2 |
Install Beta
Links
next
branchThis is complete rewrite of Reactivue based on the hooks only approach. In addition, it now supports Solid.js too. I haven't deep dived into Next.js, but it seems just work. Close #4
I deployed playground templates and a Next.js demo to Vercel. You can play with them:
First of all, its functionality depends on
setCurrentInstance
method. There is a pending PR on vuejs/core#5472 that expose this function. For now, I made apostinstall
script. I tested it withnpm pack
on out of monorepo and it worked very well.At this stage, the rewrite is pretty complete. But still, there are some topics we need to discuss.
Aliassing to Reactivue
As you can see, on the playground templates I didn't alias
vue
,@vue/runtime/core
or@vue/reactivity
to Reactivue. As we don't have our own rewrites of internal reactivity utils likewatch
, we can completely skip this extra step and make installation of Reactivue easier.Solid Support
Currently Solid.js support comes within
reactivue/solid
import. Should we publish Solidivue as a separate package?Behavioral changes
On previous version of our React implementation we were re-creating the given setup function again if props are changed. I removed that logic. Because re-creating setup state causing to lose state we modified after first render. You have to use our
onPropsChanged
helper in React andcreateEffect
in Solid to deal with prop changes.Installation step for Vue plugins are changed. Now we export a
ReactivueProvider
component that takes plugins as a prop. Same component exist for Solid.js too but for some reason it didn't work. I will look into it later. For now, I just called that function outside of render tree.Releasing
This rewrite comes with breaking changes.
How we should release it? @antfu This branch is fork of your ts starter. I just replaced
unbuild
withtsup
. Previously we were using bumpp for releasing, your ts starter has Github Action for releasing. I commented it out to let you decide how we should go. I was thinking about releasing preminor v0.5 or premajor 1.0 version with bumpp until we add Preact support. What do you think about that?TODOs:
Acknowledgments
Huge thanks to @crutchcorn for their interest in this repo. Your tests helped me to catch 1 bug 🤗
Thanks to @antfu and Vitest contributors. I just copied current jest tests, updated imports, then all tests passed except the bug I mentioned above 😅 That was the fastest testing experience I have ever had.