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

[BUG] Props object is empty, values in attrs instead #614

Open
lmiller1990 opened this issue Oct 21, 2022 · 8 comments
Open

[BUG] Props object is empty, values in attrs instead #614

lmiller1990 opened this issue Oct 21, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@lmiller1990
Copy link
Member

lmiller1990 commented Oct 21, 2022

Describe the bug

I am using Vue with TSX. The props object does not contain the property I expect. It is available on attrs.foo, but this is not what I was expecting (maybe I misunderstood how the TSX plugin works)?

Possible dup of #607. Posting in English for more 👀 .

I can debug this if it's confirmed to be an actual bug?

import { defineComponent } from "vue";

const Bar = defineComponent<{ foo: string }>({
  setup(props) {
    return () => <h1>{props.foo}</h1>
  }
})

export const App = defineComponent({
  setup() {
    return () => <Bar foo='foo' />
  }
})

Reproduction

https://github.com/lmiller1990/vue-tsx-props-bug

Steps to reproduce

yarn
yarn dev

Code is in src/App.tsx.

System Info

System:
    OS: macOS 12.2.1
    CPU: (8) x64 Apple M1 Pro
    Memory: 147.48 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.16.0/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 106.0.5249.119
    Firefox: 100.0
    Safari: 15.3
  npmPackages:
    @vitejs/plugin-vue: ^3.1.0 => 3.1.2 
    @vitejs/plugin-vue-jsx: ^2.0.1 => 2.0.1 
    vite: ^3.1.0 => 3.1.8 


### Used Package Manager

yarn

@lmiller1990 lmiller1990 added the bug Something isn't working label Oct 21, 2022
@lmiller1990
Copy link
Member Author

lmiller1990 commented Oct 22, 2022

Edit: If you add props: ['foo'], it works. I guess this is intended behavior, then? It seems unintuitive, for anyone familiar with JSX coming from React, etc.

Problem is here in core: https://github.com/vuejs/core/blob/96eb7452548293c343613ab778248a5da9619f45/packages/runtime-core/src/componentProps.ts#L182

We could do

const vals = { ...props, ...attrs }
instance.props = isSSR ? vals : shallowReactive(vals)

Merging in attrs - not sure if this would mess anything up, but it seems to work great.

Edit: Seems already discussed and denied here: vuejs/core#5730

@funny-family
Copy link

funny-family commented Oct 25, 2022

@lmiller1990, it is not a bug, it is how ts works with jsx. In order to make autocomplete work you need a trick like this one defineComponent<{ foo: string }>. See full example here. The starting point is when filename ends with .component.ts.

@lmiller1990
Copy link
Member Author

It's quite messy that we have to define it like that. This isn't the case for FunctionalComponent - everything is in props. It's noted in the docs.

If the props option is not specified, then the props object passed to the function will contain all attributes, the same as attrs.

I guess changing stateful components to work like this would break things? I wonder if we could just merge props and attrs.'

Oh, discussed here already: vuejs/core#5730. In fact it has an RFC https://github.com/pikax/rfcs/blob/no_props_component/active-rfcs/0000-no-props-component.md

I wonder if we could merge attrs in here: https://github.com/vuejs/core/blob/96eb7452548293c343613ab778248a5da9619f45/packages/runtime-core/src/componentProps.ts#L182. Then it would work how I'm (and people coming from React) would expect.

@funny-family
Copy link

funny-family commented Oct 31, 2022

@lmiller1990 this defineComponent<{ foo: string }> is just hack for ts to make autocomplete work (it is how I see this). props and attrs merge by default (see mergeProps flag in config of jsx plugin).

Edit1: FunctionalComponent !== defineComponent

@lmiller1990
Copy link
Member Author

Edit1: FunctionalComponent !== defineComponent

Yep, I noticed this... this is the main point of confusion, I guess - the same markup <Foo bar="baz" /> works differently depending on how Foo is defined, which is pretty confusing.

I don't know this is something we can really change right now, but I wonder if merging attrs into props would be a breaking change for stateful components.

@funny-family
Copy link

Edit1: FunctionalComponent !== defineComponent

Yep, I noticed this... this is the main point of confusion, I guess - the same markup <Foo bar="baz" /> works differently depending on how Foo is defined, which is pretty confusing.

I don't know this is something we can really change right now, but I wonder if merging attrs into props would be a breaking change for stateful components.

<Foo bar="baz" /> "might" work. Buuuuut... 1) We need to test it. 2) If it wont, we can apply the same principle as I did defineComponent<{ foo: string }> just need to check FunctionalComponent type.

I modified some types to make autocomplete work. That is enough for me :)

@lmiller1990
Copy link
Member Author

I might play around a bit, it's probably not worth the time/effort unless more people are interested in this, in which case we might have to make some code change in core - I don't think we can do this in the babel-plugin.

@funny-family
Copy link

Effort - yes. Time - yes^2 (all my time is here). You can play with it, because at first it does not clear how to use vue + jsx + ts. If I find the strength in myself, then I will finish the documentation about this mad Triptych.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants