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(types): add DOM type definition for JSX element #9369

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

andoshin11
Copy link

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Descriptions

By providing officially maintained HTML Attributes Types, developers can get a huge benefit when they are writing TSX.

2019-01-25 19 04 08

All you need is just a few tweaks on shims-tsx.d.ts

// shims-tsx.d.ts
import Vue, { VNode, VIntrinsicElementAttributes } from 'vue'

declare global {
  namespace JSX {
    // tslint:disable no-empty-interface
    interface Element extends VNode {}
    // tslint:disable no-empty-interface
    interface ElementClass extends Vue {}
    interface IntrinsicElements extends VIntrinsicElementAttributes {
      [elem: string]: any
    }
  }
}

@andoshin11 andoshin11 force-pushed the feature/add-jsx-element-type branch from 30f09f6 to d13720b Compare January 25, 2019 10:14
@kazupon
Copy link
Member

kazupon commented Jan 25, 2019

@andoshin11
Thank you for your PR!
It looks nice to me, but it looks like it should be provided in other ways (e.g. Vue CLI plugin), not Vue Core.

@ktsn @HerringtonDarkholme
What do you think?

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Jan 26, 2019

I think this is a nice add. However I agree with @kazupon this change is not suitable to be included in Vue's core repo. The reason is that dom definition alone cannot provide tsx improvement by default: users have to include shim-tsx.d.ts manually.

My point is if we want to include the dom definition for tsx, we should support tsx support at core repo at first (which is already scheduled in Vue 3). Otherwise this change is better to reside in tsx-support library. @ktsn any opinion?

@ktsn
Copy link
Member

ktsn commented Jan 26, 2019

I think it is fine to include this in core repo now as we'll do that sooner or later. We can update shim-tsx.d.ts in Vue CLI template to use the dom types until we support it by default.

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

Successfully merging this pull request may close these issues.

6 participants