fix(core.d.ts): export HTMLAttributes and DevJSX to fix TS4023 issue …

[gparlakov]

fix #5140

What is it?

• Feature / enhancement
• Bug
• Docs / tests / types / typos

Description

When creating a UI library for Qwik and strong typing the properties using QwikIntrinsicElement used like

export type MyLabelProp = QwikIntrinsicElement['label']

that results in a TS4023 issue.

Reproduction
https://github.com/gparlakov/qwik-lib-TS4023-issue-repro

Suggest exporting all from the jsx-generated.ts file instead of just the few types currently exported.

Previously libraries have done that like ngrx/platform#1118 or guessing what was done to fix #2783

TS issue microsoft/TypeScript#5711

Here's the real world use case from qwik-ui

• DevJSX ends needed to be exported - https://github.com/qwikifiers/qwik-ui/blob/main/packages/kit-headless/src/components/combobox/combobox.tsx#L31
• Label Attributes ends up needed to be exported - https://github.com/qwikifiers/qwik-ui/blob/main/packages/kit-headless/src/components/combobox/combobox-label.tsx

Checklist:

• My code follows the developer guidelines of this project
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• Added new tests to cover the fix / functionality

[netlify]

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	da8fb64

[mhevery]

Thanks for the fix! Could you help me better understand this. Why is this not sufficient?

import { jsxDEV, QwikIntrinsicElements } from "@builder.io/qwik/jsx-runtime";

[gparlakov]

Hey @mhevery, sure!

So why?:

• It seems QwikIntrinsicElements is not exported from the mentioned file
  
• also jsxDev is a function definition while DevJSX is a interface (with fileName etc)
• also we really need the LabelHTMLAttributes (the constituents of QwikIntrinsicElements) b/c that is what needs to be named in the d.ts file
• and finally - let's say that these were indeed exported from jsx-runtime - then how does a developer decide if they should import QwikIntrinsicElements from core (i.e.@builder.io/qwik) or @builder.io/qwik/jsx-runtime ?

[shairez]

Thanks @gparlakov !

There's a build error
Can you please run pnpm api.update and commit the changes?

[gparlakov]

Done.

I was wondering if we can have a kind of e2e test that makes sure this thing does not regress. Kind of like - get the minimal repro component and make sure it can compile the d.ts files. Any ideas?

The straightforward way would be

• a (new?) folder with d.ts focused tests
• the component that uses these fixed types
• a script that runs tsc -b aiming at d.ts only files

How does that sound?

[gparlakov]

Oh, I think I need to annotate the exported types to make the api.update run successfully.

On it.

[mhevery]

Is this ready to merge?

[gparlakov]

Sounds like the api.update makes sure that the types remain as they are now. So, on my side Yes

…

On Wed, Sep 13, 2023, 10:09 PM Miško Hevery ***@***.***> wrote: Is this ready to merge? — Reply to this email directly, view it on GitHub <#5141 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2SEV7FHGAU6546ZVESYTTX2IAIFANCNFSM6AAAAAA4SB2JYQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[shairez]

Thanks @gparlakov !

About e2e, there's a whole pacakge of e2e apps that we could add a component which uses one of the types.
We can do it on a separate PR

I'm merging so we could issue a version that fixes this asap

Thanks again!

[gparlakov]

Great, thanks!

…

On Thu, Sep 14, 2023, 4:38 PM Shai Reznik ***@***.***> wrote: Thanks @gparlakov <https://github.com/gparlakov> ! About e2e, there's a whole pacakge of e2e apps that we could add a component which uses one of the types. We can do it on a separate PR I'm merging so we could issue a version that fixes this asap Thanks again! — Reply to this email directly, view it on GitHub <#5141 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2SEVZQC3BRHTIIKNQ7K7LX2MCG3ANCNFSM6AAAAAA4SB2JYQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[shairez]

Rethinking about it @gparlakov

I see it adds TONS of stuff to the documentation
Do you think we need ALL of this? or we can constrain it to specific common types most devs will need?

[gparlakov]

I believe we do. These are the constituents of QwikIntrinsicElement (i.e. QwikIntrinsicElement['div'] and 'label' and all other html elements attributes)plus DevJSX. I think we do need them all.

…

On Thu, Sep 14, 2023, 4:41 PM Shai Reznik ***@***.***> wrote: Rethinking about it @gparlakov <https://github.com/gparlakov> I see it adds TONS of stuff to the documentation Do you think we need ALL of this? or we can constrain it to specific common types most devs will need? — Reply to this email directly, view it on GitHub <#5141 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2SEV6QGKHTQ2RQKMQQCP3X2MCSFANCNFSM6AAAAAA4SB2JYQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gparlakov]

@shairez I can think of one way to test it for yourself (+ double check my logic)

• open a component that relies on QwikIntrinsicElement['div'] (or label, li whatever)
• go to the definition
• that should take you to IntrinsicHTMLElements
• notice almost every one of these has a dedicated Attributes type(AnchorHTMLAttributes, AreaHTMLAttributes, ...)
• each of these is what I'm trying to expose (and update documentation to show it's public - example)

[mhevery]

Thought about it some more, and convinced me that this is what is already being exported, just in a weird way, so it is public....