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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 6 commits into from
Closed

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

wants to merge 6 commits into from

Conversation

znck
Copy link
Member

@znck znck commented Nov 10, 2022 •
edited by yyx990803
Loading

  • 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 •
edited
Loading

❌ 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 •
edited
Loading

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.

znck
znck commented Nov 11, 2022
View reviewed changes
test-dts/tsx-namespaced.test-d.tsx
@@ -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.

znck
znck commented Nov 11, 2022
View reviewed changes
packages/runtime-dom/types/jsx.d.ts

// 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.

znck
znck commented Nov 11, 2022
View reviewed changes
packages/jsx/package.json
@@ -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
znck
znck commented Nov 11, 2022
View reviewed changes
packages/jsx/src/h.ts
}
export interface IntrinsicAttributes extends ReservedProps {}
export interface ElementChildrenAttribute {
$slots: {}
Copy link
Member Author

@znck znck Nov 11, 2022 •
edited
Loading

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 •
edited
Loading

Also, can we use something like

/** @jsxImportSource @vue/jsx */

See jsxImportSource example:

@KaelWD KaelWD mentioned this pull request Feb 28, 2023
Closed
@haoqunjiang haoqunjiang linked an issue Mar 14, 2023 that may be