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

Make JSX typings an optional install as they conflict with React TSX #1033

Closed
elevatebart opened this issue Apr 22, 2020 · 39 comments · Fixed by #7958
Closed

Make JSX typings an optional install as they conflict with React TSX #1033

elevatebart opened this issue Apr 22, 2020 · 39 comments · Fixed by #7958
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: types ✨ feature request New feature or request

Comments

@elevatebart
Copy link
Contributor

Version

3.0.0-beta.3

Reproduction link

https://github.com/elevatebart/vue-next-react-ts-conflict

Steps to reproduce

# Checkout the repo
git checkout https://github.com/elevatebart/vue-next-react-ts-conflict.git

# Install all modules
npm ci

# run tsc
npm start

The errors you see in the console are due to React and Vue TSX accepting different values.

What is expected?

When migrating from React to Vue, types should not fail without installing them.

What is actually happening?

it fails at compilation


JSX types could be made a separate package: @vue/jsx-types.

This way, types would not systematically be installed.

Devs could install the runtime they need without the typings.

The transition would be easier.

@elevatebart
Copy link
Contributor Author

I could see this problem be solved temporarily by resynchronizing the types of react and vue.
I would see this alignment as a nightmare to maintain in the long term.

@yyx990803
Copy link
Member

But are you going to use JSX/TSX in Vue, or are you going entirely with templates?

While this does make it compile, you still wouldn't be able to use Vue TSX until you completely remove React typings?

@elevatebart
Copy link
Contributor Author

Agreed, and choose when to do this switch from React JSX to Vue JSX.

What do you suggest?

a bit of context

Vue-styleguidist uses React to render a style guide for VueJs components. Most of the code is in react though. Vue only comes in play in the preview. The reason being, same as for storybook, that the original product was written in react.

Should I rewrite it completely using Vue? Though there is a project started for that, I feel like I am letting my users down changing technology suddenly. Since all customizations of styleguidist was done in react before, it would be better to be able to at least choose when they switch.

Really looking forward to your opinion.

@elevatebart
Copy link
Contributor Author

I could create a Pull Request to expose what I think could be useful.

@elevatebart
Copy link
Contributor Author

elevatebart commented Apr 23, 2020

@shilman, @Aaron-Pool Did you experience the same kind of issue implementing vue-next in storybook?

@elevatebart
Copy link
Contributor Author

The main conflict seems to be about NativeElements/ IntrinsicElements.

Another solution might be to share the definition of the NativeElements in a separate package, then have React and Vue typings depend on the same package.

This way, TSX conflicts will be avoided until one chooses to switch from react TSX to vue TSX.

@shilman
Copy link

shilman commented Apr 23, 2020

@elevatebart we haven't yet implemented vue-next in storybook. you're in uncharted territory! 😄

@Aaron-Pool
Copy link

@elevatebart Same here, the furthest I've gone is using jsx with the Vue 2.x composition-api plugin. Haven't started to explore the vue-next waters just yet.

@elevatebart
Copy link
Contributor Author

Hello @DanielRosenwasser,

Does TypeScript have a solution to deal with JSX ambient types conflicting?
I tried using the "types":[] trick without luck.
I tried using the paths: {} trick to redirect retrieval of types to another object but since types are ambient there is still a conflict.

What solutions do you see?

@deleonio
Copy link

deleonio commented May 2, 2020

Either vue or react not both.
That is the same issue by use react and preact together.

Use storybook for your components doc.

@elevatebart
Copy link
Contributor Author

@martinoppitz I do not understand. Do you mean that preact typings are different from react typings?

What do you suggest I do in a monorepo that has both react and Vuejs ?

@elevatebart
Copy link
Contributor Author

React and VueJs are not in the same package in the above monorepo.

@deleonio
Copy link

deleonio commented May 2, 2020

I think the ide does not differnce beetween the subfolder projects.

Thank you for this perspective ... because I believe/feel "mono repos are a other name for monoliths."

try to reduce the overhead by managing the independent packages ... a go back to move all in only one repo.

If you are disciplined, then you can also properly design/develop monoliths.

@elevatebart
Copy link
Contributor Author

Thank you @martinoppitz,

I understand your disliking for monorepos. They can create monoliths that are long to clone and even longer to install.

In our case though, the vue-next monorepo could save our bacon. It allows easy extraction of a package in a PR. Once the PR is accepted, all 3 modified packages are delivered at once. No downtime when packages are incompatible or missing a library and e2e testing can continue without breaking.

Regarding JSX, can you expand a little what I should do?
Are you implying that this is not an issue?
Are you suggesting a maintainer should use one repository for React and one for Vue and they should never mix?

I would not expect The Progressive JavaScript Framework to force a complete rewrite of the code upon adoption if there is another solution.

@yyx990803
Copy link
Member

@elevatebart just so you know I'm considering doing this, but still trying to figure out the best way to organize it with the upcoming Vue 3 JSX transform.

@elevatebart
Copy link
Contributor Author

Thank you @yyx990803, this will help a lot.
If you are looking for someone to bounce ideas with, ping me on VueLand.
I'll be happy to show enthusiasm or constructive criticism.

Except for this hiccup, the migration to Vue 3 is bringing a lot of performance to docgen and styleguidist. I like it.

@wonderful-panda
Copy link
Contributor

FYI, namespace JSX can be renamed to avoid conflict (for example, VueJSX ).

And TypeScript uses VueJSX instead of JSX by putting this configuration into tsconfig.json .

"jsxFactory": "VueJSX"

@lcoder
Copy link

lcoder commented Jun 22, 2021

I create a vue project with vue-cli. the project has a dependency of package named B. B has a dependency with react types.
for example:

import type { PropsWithChildren } from 'react';

then my project emit errors:

Type '{ children: Element[]; class: string; }' is not assignable to type 'DetailedHTMLProps<HTMLAttributes, HTMLDivElement>'.
Property 'class' does not exist on type 'DetailedHTMLProps<HTMLAttributes, HTMLDivElement>'. Did you mean 'className'?ts(2322)

here is my code:
index.tsx:

import { defineComponent, ref } from 'vue';
import style from './index.module.less';

export default defineComponent({
  setup() {
    const activeTab = ref('tab-one');
    return () => (
      <div
        class={style.detail}
      >
        xxxx
      </div>
    )
  },
});

I strive for solving this, what can I do with this situation?

remove the package which has a dependency with react library, the error disappear.

by the way. I organized my project with mono repo. which has react project and vue project.

@olemarius
Copy link

I started getting these errors after installing storybook 3.6.10. My project's stack is vite + vue3 (typescript), while storybook is using webpack and is based on react, so I'm making a wild guess that by adding storybook, these errors started surfacing because of react dependencies.

Type '{ class: string; }' is not assignable to type 'DetailedHTMLProps<OlHTMLAttributes, HTMLOListElement>'.\n Property 'class' does not exist on type 'DetailedHTMLProps<OlHTMLAttributes, HTMLOListElement>'. Did you mean 'className'?"

@filipecruz
Copy link

@olemarius did you find more info on this?

@znck
Copy link
Member

znck commented Oct 18, 2021

I have a monorepo with one package depending on Vue and other on react. I use pnpm which does not hoists dependencies. Then I have a tsconfig.json for each package. Hence, no JSX conflicts.

But If I have to use Vue and react in same package/file, avoiding conflict is not possible.

@lauragift21
Copy link

I started getting these errors after installing storybook 3.6.10. My project's stack is vite + vue3 (typescript), while storybook is using webpack and is based on react, so I'm making a wild guess that by adding storybook, these errors started surfacing because of react dependencies.

Type '{ class: string; }' is not assignable to type 'DetailedHTMLProps<OlHTMLAttributes, HTMLOListElement>'.\n Property 'class' does not exist on type 'DetailedHTMLProps<OlHTMLAttributes, HTMLOListElement>'. Did you mean 'className'?"

@olemarius Did you find a way to resolve this I'm having the same problem with Vue and Storybook setup with vite + vue3?

@zhang-stone
Copy link

I had the same problem

XavierChevalier pushed a commit to XavierChevalier/jam that referenced this issue Nov 26, 2021
Refs: //github.com/storybookjs/storybook/pull/16629,//github.com/storybookjs/storybook/pull/16630,//github.com/vuejs/core/issues/1033,//github.com/storybookjs/storybook/issues/12505,//github.com/XavierChevalier/ewokify-app/runs/4326759026?check_suite_focus=true
@dcy0701
Copy link

dcy0701 commented Dec 2, 2021

same problem @yyx990803

@olemarius
Copy link

olemarius commented Dec 13, 2021

Hi,

Try this in your tsconfig.json file

 "exclude": [
        "**/node_modules",
        "**/dist",
        // https://github.com/johnsoncodehk/volar/discussions/592
        "**/*.stories.ts"
    ]

@abhic91
Copy link

abhic91 commented Dec 14, 2021

Thanks olemarius.

Adding this fixed it

// tsconfig.json
{
  "compilerOptions": {
    // ...
    "types": [
      "vite/client", // if using vite
      // ...
    ]
  }
}

@fobdy
Copy link

fobdy commented Jan 3, 2022

For me latest @vue/runtime-dom is also conflicting internally.
excluding **/*.stories.ts and including vite/client doesn't prevent @vue/runtime-dom having conflict with @types/react. I have no idea where VS Code's TS might grab the React types, but we have @storybook/vue installed and some *.ts storybook stories in the project.

@wadadanet
Copy link

I deleted the @storybook/testing-vue3": "^0.0.1 and the error stopped.
Removing node_modules/@storybook/testing-vue3/ had the same effect.

@PuruVJ
Copy link

PuruVJ commented Feb 11, 2022

Having same issue, but because I have a react and vue package in packages folder in a pnpm workspace, and it's by design

Svelte typings don't break though
CleanShot 2022-02-11 at 15 10 35@2x

Edit: It's resolved for me. Had to provide "preserveSymlinks" in tsconfig.json for rollup-plugin-dts to work

@K3TH3R
Copy link

K3TH3R commented Feb 28, 2022

I was playing with Storybook's new Interaction Testing (Vue/Vite builder) and I started seeing these issues about JSX as well in my Vue components. Upon uninstalling the dependencies (and re-installing the node_modules folder) the errors disappeared for me as well.

sibbng added a commit to antfu/reactivue that referenced this issue Mar 5, 2022
sibbng added a commit to antfu/reactivue that referenced this issue Mar 8, 2022
@Andarist
Copy link

Andarist commented Mar 9, 2022

There is actually a way to make JSX namespace "local". It could be a namespace attached to the h factory, similarly to what I've done in Emotion:
https://github.com/emotion-js/emotion/blob/4266aa0183fe2c19904c25c6e803e8c534923865/packages/react/types/index.d.ts#L96-L114

I've explored this quite a bit and I'm also advocating for @types/react to use this technique instead of polluting the global JSX namespace but I'm not sure when this stuff might land there (well, not sure if it ever lands there).

I'm happy to discuss this further if you'd like to resolve this here once and for all, I could even prepare a PR for this. I'm not a Vue user though and I lack certain expertise when it comes to understanding the butterfly effect of such a change. So if the team here would be willing to discuss this - let me know and we could go through this together to recheck if such a change could be introduced in a backwards-compatible manner.

@elevatebart
Copy link
Contributor Author

With the lack of traction on this issue, I think we should be OK closing it.

The brittle workaround is to use patch-package to remove the JSX global declarations that we do not want.

It works, with the caveat that every time we update vue, we have to re-write the patch.

PNPM has an equivalent pnpm patch that can help too if you are using pnpm.

@Andarist
Copy link

Andarist commented Jul 6, 2022

IMHO the issue should stay open because it's still valid. As mentioned in the past - I'm willing to work on this if somebody green lights this. I would probably also need to chat with the maintainers before getting to work because we'd need to establish the risks, breakage concerns, potential migration strategies etc

@zoy-l
Copy link

zoy-l commented Aug 12, 2022

guys ,add at the head of the file

/* @jsxImportSource @vue/runtime-dom */

or

tsconfig.json
{
...
  "types": ["vue"],
}

I test it works

@elevatebart
Copy link
Contributor Author

Thank you @zoy-l i did not know about this "trick". Will try it out.

I ended up segregating the two code-bases using tsconfig reference.

@alecgibson
Copy link

In our project, we're using TSX files as a nice way to write vanilla HTML outside of the Vue ecosystem (for performance reasons), which — not incorrectly — uses a syntax which is incomaptible with Vue syntax.

The Vue docs even state:

Although first introduced by React, JSX actually has no defined runtime semantics and can be compiled into various different outputs.

Shouldn't it follow that vue shouldn't automatically clobber the global namespace by imposing semantics on a semantically undefined syntax?

@ChibiBlasphem
Copy link

Using this thread to remind that because of those jsx types clobbering the global namespace (on vue 2.7.13 at least), React and Vue cannot be used on the same project which is a real problem.

@datalater
Copy link

Using this thread to remind that because of those jsx types clobbering the global namespace (on vue 2.7.13 at least), React and Vue cannot be used on the same project which is a real problem.

That's exactly what happened to me as well. As below, React and Vue have different global namespace JSX types. And that's where type collision occurs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: types ✨ feature request New feature or request
Projects
None yet