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

spread() doesn't use the children property when an Accessor is passed #2342

Open
AFatNiBBa opened this issue Oct 18, 2024 · 6 comments
Open
Labels
typescript relating to typescript or types

Comments

@AFatNiBBa
Copy link

AFatNiBBa commented Oct 18, 2024

Describe the bug

The type definition of the spread() function states that it also accepts a function that returns an object instead of the object itself, but here it doesn't check if props is a function when accessing the children property.
After checking with the playground, it seems that no property works

Your Example Website or App

https://playground.solidjs.com/anonymous/8afd997e-2827-47bf-b670-bcfbd3825315

Steps to Reproduce the Bug or Issue

Just look at the output:

  • "This works" is visible
  • "This doesn't" should be visible but isn't

Expected behavior

It should get the properties from the returned object if props is a function OR the type definition should be made to match the actual behaviour by disallowing Accessors

Screenshots or Videos

No response

Platform

  • OS: Windows 11
  • Browser: Edge 129.0.2792.89
  • Version: 1.9.2

Additional context

May be related to #1296

@ryansolid
Copy link
Member

ryansolid commented Oct 18, 2024

99% this is a types issue. We use this internally where everything is getters. We don't special case functions here I believe. But it probably is worth double checking.

@mdynnl
Copy link
Contributor

mdynnl commented Oct 18, 2024

I remember where <div {...func} /> and <div {...func()} /> compiled to the same code and stopped working at some point which is ryansolid/dom-expressions@12e84da. As children, ref is handled separately from the rest of the props, spread can't accept accessor anymore so the compiler assists by wrapping with mergeProps to turn it into getter object. But children can still be a getter or an accessor though.

After that commit https://github.com/solidjs/solid/releases/tag/v1.6.0 happened.

@AFatNiBBa
Copy link
Author

(Just to be clear: In my code I have absolutely no need to pass an Accessor to spread(), I just ran into the problem)

@mdynnl
Copy link
Contributor

mdynnl commented Oct 19, 2024

It should get the properties from the returned object if ...

Right, I missed this section.

I just ran into the problem

Not clear what this means though. Is it some external/internal code or a template that outputs this?

@AFatNiBBa
Copy link
Author

Not clear what this means though. Is it some external/internal code or a template that outputs this?

I just tried to use the function as an experiment

@ryansolid
Copy link
Member

Thanks @mdynnl I think you are correct. I realized that spread as a function was insufficient and changed to only accepting objects. So I think that confirms it is a types thing.

@ryansolid ryansolid added the typescript relating to typescript or types label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript relating to typescript or types
Projects
None yet
Development

No branches or pull requests

3 participants