Skip to content
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

Composition API in a functional component #1645

Closed
baryla opened this issue Jul 19, 2020 · 14 comments
Closed

Composition API in a functional component #1645

baryla opened this issue Jul 19, 2020 · 14 comments

Comments

@baryla
Copy link

baryla commented Jul 19, 2020

Version

3.0.0-rc.2

Reproduction link

https://github.com/baryla/vue3-composition-api-functional-component

Steps to reproduce

  1. Run the yarn dev command
  2. Click on the button

What is expected?

I would expect the counter to increment

What is actually happening?

The counter is not incrementing


I'm not exactly sure if this is the desired behaviour but it feels a little strange that this doesn't work.

If it desired and Composition API should only work inside setup, maybe it's worth exploring the use inside a functional component and treating the whole function as a setup function? Kind of similar to this RFC where the whole script tag is considered a "setup" if it has the setup attribute.

@CyberAP
Copy link
Contributor

CyberAP commented Jul 19, 2020

Functional components do not have state so this should be expected.

@baryla
Copy link
Author

baryla commented Jul 19, 2020

@CyberAP Yeah that totally makes sense. Just makes me curious whether there would be any major performance impacts in having functional components be stateful.

@icehaunter
Copy link

@baryla I believe they state in the docs that performance difference is negligible for functional components, so Vue 3 migration guide recommends using stateful components

@baryla
Copy link
Author

baryla commented Jul 20, 2020

@icehaunter thanks for the link buddy. Yeah I read that part which is fair enough but I thought that maybe we can remove that extra indentation that we have with setup and potentially have functional components be stateful - kind of like what we see with React and it's functional components and hooks. No big deal at all if this isn't the direction that Vue team wants to go.

@bjarkihall
Copy link
Contributor

I thought functional components would support the Composition API, according to rfc0008.
So is inject only reactive but not refs?

@bjarkihall
Copy link
Contributor

bjarkihall commented Jul 20, 2020

Right now you can either define your refs in a higher scope - putting const counter = ref(0) in module scope raises the counter, since the function of the FC is invoked whenever the ref updates.

So this doesn't work:

function useCounter() {
  const counter = ref(0);
  function increment() {
    counter.value++;
  }
  return { counter, increment };
}

while this actually does:

const counter = ref(0);
function useCounter() {
  function increment() {
    counter.value++;
  }
  return { counter, increment };
}

This has two issues though:

  1. The counter ref would be shared with all instances of this component and while you could create a mapper for the component instance and "its refs", I think you'd just be doing what most people expect vue was supposed to be doing.
  2. useCounter is invoked for each render, which also defines increment each time, etc. This explains why the ref is always set to 0, you can actually log inside your FC to see the function is being run each time you invoke the increment function.

I wonder why this isn't possible then (returning a function which returns the vnode), just like setup does:

const Counter = () => {
  const { counter, increment } = useCounter()
  return () => <button onClick={increment}>Counter: {counter.value}</button>
};

But FunctionalComponents don't seem to support that, so it won't work.
I saw this comment about wrapping the FC with a helper:

const defineSetupComponent = (setupFn) => defineComponent({ name: setupFn.name, setup: setupFn })
export default defineSetupComponent(Counter);

which works, but I still think all of this will confuse a lot of developers, especially those coming from React.

@bjarkihall
Copy link
Contributor

Shouldn't this at least be caught by eslint and be documented to avoid confusion?

@CyberAP
Copy link
Contributor

CyberAP commented Jul 20, 2020

@bjarkihall if you need to simplify your component you can use setup to return a render function, instead of creating a Functional Component for that:

export default {
  setup() {
    const counter = ref(0)
    const increment = () => counter.value++
    
    return () => [
      h('div', counter),
      h('button', { onClick: increment })
    ]
  }
}

@AjaiKN
Copy link
Contributor

AjaiKN commented Jul 20, 2020

If you don't care about having the name field, I think defineComponent already wraps a function into setup if you pass it a function directly: https://github.com/vuejs/vue-next/blob/d39c03771b7ae2eb6026c3463a5c0972767780ff/packages/runtime-core/src/apiDefineComponent.ts#L207-L209

So it seems like you could do this to define a stateful component without making your own defineSetupComponent function:

export default defineComponent(() => {
  const { counter, increment } = useCounter();

  return () => <button onClick={increment}>Counter: {counter.value}</button>;
})

@baryla
Copy link
Author

baryla commented Jul 20, 2020

@bjarkihall that's very interesting indeed and I totally agree that it may be confusing for React devs wanting to try Vue but it's all down to the direction of Vue. Very nice writeup though, thanks for the investigation!

@Aurelius333 oh that's quite nice! So if we already have that ability, it may not be too much work to update the core to handle a stateful functional component.

@aztalbot
Copy link
Contributor

aztalbot commented Jul 20, 2020

I wonder why this isn't possible then (returning a function which returns the vnode), just like setup does:

@bjarkihall ... If I understand what you are suggesting, I remembered this had been considered in the past before settling on the approach of using defineComponent.

vuejs/rfcs#42 (comment)
vuejs/rfcs#42 (comment)

@bjarkihall
Copy link
Contributor

@Aurelius333 I didn't notice that, thanks! I'd think the "name" mostly matters when using devtools, so maybe the implementation could use if(__DEV__) to get the name of the function, it's treeshaken away after all, right?
@CyberAP I know, the Functional Components were not clear (stateless vs stateful) - they're kind of like the returned function in setup, instead of being like the setup function itself - all three functions receive the (props,ctx) arguments, which might be the confusing part, since the render option property doesn't receive it but behaves similarly.

So this is kind of the functionality of Functional Components:

const FC = ({counter, increment}) => (props, ctx) => <button onClick={increment}>Counter: {counter.value}</button>
export default {
  setup(props, ctx) {
    return FC(useCounter()) // don't do this though...
  }
}

instead of this:

const FC = (props, ctx) => {
  const {counter, increment} = useCounter();
  return <button onClick={increment}>Counter: {counter.value}</button> // this would need to return a render function to work!
}
export default {
  setup: FC
}

So it actually boils down to this:
Stateful "component": (props, ctx) => () => vNode;
Stateless "component": (props, ctx) => vNode;

As I said, there should be clearer notes about this in the docs and a maybe a rule that catches this.
The FunctionalComponent type looks like this: (props: P, ctx: SetupContext<E>): any; while defineComponent accepts setup: (props, ctx) => RawBindings | RenderFunction, where RenderFunction: () => VNodeChild.
Shouldn't the FunctionalComponent type just return vNodeChild too, for starters?

EDIT: @aztalbot thanks for the links! I had already written this up before seeing the link where Evan addresses this.

@yyx990803
Copy link
Member

The type of a component (stateful vs. functional) must be known before hand because the side-effect of calling

function Counter(props) {
  // the render function itself
  return h('div', props.foo)
}

vs.

function Counter(props) {
  // stateful setup logic

  return () => h('div', props.foo)
}

is completely different. The former is expected to be inside a reactive effect that tracks its dependencies. The latter does not. So it would be too late to determine how this function should be treated based on its return value.

So you either force all functional components to return the render function (even for state-less ones, which becomes confusing), or require explicit defineComponent wrapper to indicate this is a stateful component. I'd rather go the explicit route.

I think this sort of confusion largely comes from React hooks users, but in Vue 3 the rule of thumb is: functional components are always state-less. Use an object or defineComponent if you want a stateful component.

@bjarkihall
Copy link
Contributor

@yyx990803 it makes total sense, but what doesn't make sense is that defineComponent type is an intersection of both ComponentPublicInstanceConstructor and FunctionalComponent type - since defineComponent doesn't ever return a FunctionalComponent in the implementation.

Also, can't JSX.Elements have a stricter type (instead of accepting any), which enables types for:

interface VueElement { ... } // an interface for JSX.Element which resembles VNode
declare global { namespace JSX { interface Element extends VueElement { } }
StatelessFC: (props?, ctx?) => VueElement // a stricter render function

and

StatefulFC: (props?, ctx?) => StatelessFC // a stricter setup function

This would hopefully allow us to make these much clearer in both places, since as of now, TS won't detect errors because of FunctionalComponent: (props,ctx)=>any and defineComponent's RawBinding for the setup function (which doesn't really validate against the RenderFunction type, so it's basically functioning like any).

VNodeChild is not compatible with JSX.Element, so e.g. () => 0 (or the use of any primitive type, sadly), so it wouldn't be valid as a component. I noticed that tests in functionalComponent.test.d.tsx started failing once I started experimenting with these - but then again, the same tests would fail in e.g. React, since () => 0 is not a valid component for JSX (for the same reason).

Since primitives are supposed to work in a non-JSX context - could the VNodeChild be used for more general components but the more limited/narrow VueElement types in JSX context?

@github-actions github-actions bot locked and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants