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

feat(jsx-types): namespace JSX types #7083

Closed
wants to merge 6 commits into from
Closed

Conversation

znck
Copy link
Member

@znck znck commented Nov 10, 2022

  • Extract JSX types to @vue/jsx
  • Support @jsx pragma
    /** @jsx h */
    import { h } from '@vue/jsx'
  • Support global JSX types using opt-in package @vue/jsx/register
  • Avoid global JSX pollution

Related #1033 #1034

@netlify
Copy link

netlify bot commented Nov 10, 2022

Deploy Preview for vuejs-coverage failed.

Name Link
🔨 Latest commit f1f9527
🔍 Latest deploy log https://app.netlify.com/sites/vuejs-coverage/deploys/636dba7b3213660008493e65

@znck
Copy link
Member Author

znck commented Nov 10, 2022

Looks like dts tests run before building, trying to fix it.

@znck znck changed the title feat(jsx): namespaced JSX types extracted as separate package feat(jsx-types): namespace JSX types Nov 10, 2022
@znck znck added the need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. label Nov 10, 2022
@yyx990803
Copy link
Member

yyx990803 commented Nov 11, 2022

Notes:

If we land this, to avoid disruption in userland we need to:

  • IDE: Volar needs to generate TSX with /** @jsx h */ import { h } from "vue/jsx" /cc @johnsoncodehk
  • @vue/tsconfig should register globally by default so it can work with other IDEs out of the box? /cc @sodatea

This is still technically a breaking change, especially for TSX users.

To minimize the disruption, I think we should first start shipping this with auto registration by default in a minor, to allow Volar and existing projects to start using explicit global registration in advance. Then we remove auto global registration in a future release.

@@ -0,0 +1,69 @@
/** @jsx h */
import { h } from '../packages/jsx'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vue/jsx instead of ../packages/jsx does not work, don't know why.


// suppress ts:2669
export {}
import '@vue/jsx/register'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This produces a warning that ./dist/jsx does not exist. Since we are building in parallel, it is eventually created.

@@ -0,0 +1,35 @@
{
"name": "@vue/jsx",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope it's okay to use @vue/jsx as package name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more like @vue/tsx though 😂
Unless we are planning to add more functionalities to this package later.

@znck znck removed the need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. label Nov 11, 2022
}
export interface IntrinsicAttributes extends ReservedProps {}
export interface ElementChildrenAttribute {
$slots: {}
Copy link
Member Author

@znck znck Nov 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this count as breaking change?

ElementChildrenAttribute sets the name of children within props. https://www.typescriptlang.org/docs/handbook/jsx.html#children-type-checking

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vuetify uses $children. Volar uses $slots but it isn't inside $props so doesn't count.

@xiaoxiangmoe
Copy link
Contributor

xiaoxiangmoe commented Dec 2, 2022

Also, can we use something like

/** @jsxImportSource @vue/jsx */

See jsxImportSource example:

@sodatea sodatea linked an issue Mar 14, 2023 that may be closed by this pull request
@yyx990803 yyx990803 added the ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. label Mar 24, 2023
@johnsoncodehk
Copy link
Member

Also, can we use something like

/** @jsxImportSource @vue/jsx */

This works. I think users should be required to explicitly specify it in tsconfig compilerOptions instead of implicitly adding it automatically in virtual code.

The PR seems to have issues with "jsxImportSource": "@vue/jsx", adding the following file fixes it.

// packages/jsx/jsx-runtime.d.ts
import { h } from './dist/jsx'

export namespace JSX {
  interface Element extends h.JSX.Element { }
  interface ElementClass extends h.JSX.ElementClass { }
  interface ElementAttributesProperty
    extends h.JSX.ElementAttributesProperty { }
  interface IntrinsicElements extends h.JSX.IntrinsicElements { }
  interface IntrinsicAttributes extends h.JSX.IntrinsicAttributes { }
  interface ElementChildrenAttribute extends h.JSX.ElementChildrenAttribute { }
}

@yyx990803
Copy link
Member

yyx990803 commented Mar 26, 2023

Closing in favor of #7958 - restructured to vue/jsx-runtime so that it can be used with the more ergonomic @jsxImportSource, also not relying on importing a transitive dep (which can be problematic with strict package managers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. scope: types
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Make JSX typings an optional install as they conflict with React TSX
6 participants