reactivue icon indicating copy to clipboard operation
reactivue copied to clipboard

refactor!: rewrite

Open sibbng opened this issue 2 years ago • 10 comments

Install Beta

npm i reactivue@beta @vue/runtime-core

Links


This 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:

  • React https://reactivue.vercel.app/
  • Solid.js https://solidivue.vercel.app/
  • Next.js https://reactivue-nextjs.vercel.app/

First of all, its functionality depends on setCurrentInstance method. There is a pending PR on https://github.com/vuejs/core/pull/5472 that expose this function. For now, I made a postinstall script. I tested it with npm 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 like watch, 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 and createEffect 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.

  • No alias
  • No auto effect to props
  • Different installation step for plugins
  • No Preact support by direct

How we should release it? @antfu This branch is fork of your ts starter. I just replaced unbuild with tsup. 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.

sibbng avatar Mar 01 '22 00:03 sibbng

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

crutchcorn avatar Mar 01 '22 00:03 crutchcorn

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 setCurrentInstance. But postinstall hook just worked really well and I changed my mind. @antfu using same approach for vue-demi. I don't think we have to wait for official support before releasing this.

sibbng avatar Mar 01 '22 00:03 sibbng

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.

antfu avatar Mar 01 '22 04:03 antfu

Now, defineComponent support omitting render function. So, you can return your React component within a computed and use your refs directly without duplicating returned expression on render function. This is React only feature.

export const App = defineComponent(() => {
  const counter = ref(0)

  return computed(() => (
    <div>
      <p>Count: {counter.value}</p>
      <button onClick={() => (counter.value += 1)}>Increment</button>
    </div>
  ))
})

sibbng avatar Mar 03 '22 18:03 sibbng

I like the new interface, and well done! I am fine with postinstall approach (probably hard to make Vue expose them), but I think adding a section about How it works with the mention of we how and why we are using postinstall would be better.

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!

antfu avatar Mar 29 '22 22:03 antfu

Is there some body can merge this PR ?

LancelotLewis avatar May 03 '22 13:05 LancelotLewis

Is there any work left to do on this? Would love to help out if needed. @sibbng

nVitius avatar Oct 28 '22 19:10 nVitius

  • I'm not happy about the way we patch vue. It does require side-effects-cache=false and shamefully-hoist=true in .npmrc file for pnpm. It would be nice to have a standalone build that works like 0.x version.

  • Thinking of removing Solid implementation as it is not on par with React and an extra mental overhead.

  • The way Babel plugin work is too cryptic. It transforms anonymous functions to defineComponent calls.

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.

  • PROPS_UPDATED hook makes component rerendering significantly slower. Maybe reactivue components should be pure and shouldn't depend on props. I'm open for alternative suggestions.
  • We should make it clear that Reactivue is not built to be used in production apps.

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 next branch. I will do my to review it.

sibbng avatar Oct 29 '22 00:10 sibbng

Would be possible to omit the return computed() and allow to just return the render function return () => , which can then be wrapped on a computed automatically?

Dav3rs avatar Apr 12 '23 13:04 Dav3rs

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

CJY0208 avatar Apr 28 '23 02:04 CJY0208