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

Dynamic returns a function, contrary to its Typescript signature (as of v1.7) #1763

Closed
AlexErrant opened this issue Jun 6, 2023 · 3 comments

Comments

@AlexErrant
Copy link
Contributor

AlexErrant commented Jun 6, 2023

For reasons [1] [2], I want to invoke Dynamic as a function, not as JSX, like so:

const Child: VoidComponent = () => {
  return <div>Child</div>;
};

const dynamic = Dynamic({
  get component() {
    return Child;
  },
});
console.log(typeof dynamic) // function
console.log(dynamic()) // HTMLDivElement

Typescript says dynamic is of type JSX.Element, when really it is a function whose name is "bound readSignal".

More details that are probably not helpful but I spent too long writing to not include. The same happens if I wrap `Dynamic` in a `createComponent`:
const createDynamic = createComponent(Dynamic, {
  get component() {
    return Child;
  },
});
console.log(typeof createDynamic) // function
console.log(createDynamic()) // HTMLDivElement

This does not occur for ordinary Components:

const ParentComp: VoidComponent<{
  child: VoidComponent;
}> = () => {
  return <div>ParentComp</div>;
};
const createParentComp = createComponent(ParentComp, {
  get child() {
    return Child;
  },
});
const parentComp = ParentComp({
  get child() {
    return Child;
  },
});
console.log(createParentComp) // HTMLDivElement
console.log(parentComp) // HTMLDivElement

Playground demoing Typescript errors: https://playground.solidjs.com/anonymous/1eaedcbb-4e59-445b-84cf-00b7792a983c

This change to Dynamic was introduced in this v1.7 commit/PR, when Accessor<JSX.Element> was removed from the return type and as unknown as JSX.Element was added. The reasoning for this is unclear to me; the commit comment was "Remove FunctionElement from JSX.Element types". What work would need to be done to not lie to Typescript?

@AlexErrant AlexErrant changed the title Dynamic returns a function, contrary to its Typescript signature Dynamic returns a function, contrary to its Typescript signature (as of v1.7) Jun 6, 2023
AlexErrant added a commit to AlexErrant/solid-plugin-host that referenced this issue Jun 6, 2023
@andersk
Copy link

andersk commented Jun 19, 2023

<Dynamic> must return a signal (i.e. a function) rather than an HTMLElement, because component could itself be a signal that changes which element should be rendered over time. The same is true of other control flow components like <Show> and <Switch>, and of any component that uses one of those at top level. So it will always be possible for something typed JSX.Element to be a function at runtime.

The reason functions were removed from the JSX.Element TypeScript type is explained under Stricter JSX Elements in the changelog, and Ryan went into much more detail on his stream. In short, the goal was to catch mistakes where the user wraps JSX arguments and children in unnecessary (non-signal) functions, with the tradeoff that some necessary functions must now be casted to satisfy TypeScript.

@ryansolid
Copy link
Member

I heard there may be ways to address this differently in TypeScript now. But this is as intended. If you use it directly you need to cast.

Since our release this was added to TS which might help us? microsoft/TypeScript#51328

I'm more the than willing to see if there is a way so that we can handle both sides a bit differently but I think that is general approach thing and I am going to close this one as by design for now.

@ryansolid ryansolid closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2023
@AlexErrant
Copy link
Contributor Author

Ah whoops yeah forgot to close. Got sucked into that (excellent) 5hr stream.

Also linking this discussion for any future searchers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants