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

TS: Move JSX namespace to preact one #1036

Closed
marvinhagemeister opened this issue Mar 16, 2018 · 21 comments · Fixed by #1448
Closed

TS: Move JSX namespace to preact one #1036

marvinhagemeister opened this issue Mar 16, 2018 · 21 comments · Fixed by #1448
Assignees

Comments

@marvinhagemeister
Copy link
Member

Just read the changelog for TypeScript 2.8 and they finally allow mixing different JSX namespaces. This improves interoperability when preact is used alongside react components.

Note that we should not release this until TS 2.8 is out.

TS 2.8 Changelog: https://blogs.msdn.microsoft.com/typescript/2018/03/15/announcing-typescript-2-8-rc/

@developit
Copy link
Member

Nice - I saw a Twitter thread about this last week. Is there anything we can do to account for the JSX.Element change?

@marvinhagemeister marvinhagemeister changed the title TS: Move JSX namespace to preach one TS: Move JSX namespace to preact one Mar 28, 2018
@marvinhagemeister
Copy link
Member Author

FYI: TypeScript 2.8 was just released earlier today 🎉 Will tackle this issue sometimes in the next days.

@marvinhagemeister
Copy link
Member Author

Just gave this a go, but to properly support this users will have to change their code.

before:

// tsconfig.json
{
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "h",
  }
}
import { h, Component } from "preact";

export class Foo extends Component {
  render() {
    return <div>hey</div>;
  }
}

after:

// tsconfig.json
{
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "preact.h",
  }
}
// wildcard import is needed
import * as preact from "preact";

export class Foo extends preact.Component {
  render() {
    return <div>hey</div>;
  }
}

But before proceeding I'd love to see if there is a better option. Filed an issue over at the TypeScript: microsoft/TypeScript#23762

@marvinhagemeister
Copy link
Member Author

marvinhagemeister commented Apr 30, 2018

@andrewiggins I'd love to here your opinions on this. My guess is that we'll have to advice users to switch to wildcard exports sooner or later once more JSX-specific features like Fragments land.

List of exports:

  • h / createElement
  • JSX namespace (TS 2.8)
  • Fragment (not implemented yet)

@andrewiggins
Copy link
Member

Yea, that makes sense. If we require TS users to use the wildcard syntax over the destructed syntax, perhaps we make this typings change as part of the 9.0 release (if one is planned)?

@marvinhagemeister
Copy link
Member Author

I'm a bit on the fence if we should mark this as breaking or not. From the TS user perspective this is the right thing to do, but most of our users don't use TS but rely on plain js.

Perhaps we can ease the transition to a wildcard-like syntax by exposing JSX through both top-level and under h. I'll check if this is a feasible thing to do.

@andrewiggins
Copy link
Member

It should be doable. Wesley from the TypeScript suggested to do that in the issue you opened.

The question I still have is how would Fragment work. Does it need to be exported on the same object JSX is exported? I haven't played with fragments at all yet though so I need to learn more about them (and check out your PR)

@marvinhagemeister
Copy link
Member Author

@andrewiggins yep, the Fragment definition has to be exported on the JSX object. The fragments PR is still a work in progress, though :)

@lukelindsey
Copy link

lukelindsey commented Sep 12, 2018

Any update on this? Or are there any decent workarounds I can make on my side if I want to use react and preact in the same project?

All the workarounds I can think of seem quite messy. Seems like I will need to move the preact code out to have a separate package.json.

Edit: Ah, now I see in another issue (of course right after I decide to post) that we can remove the react typings to get it compiling without errors. I'm sure there's some mismatches between the two JSX typings (allowing class is the obvious one), so I'll be quite pleased when this is resolved. If theres another workaround, I'd love to hear that as well.

@cromefire
Copy link

Wouldn't "reactNamespace": "preact" work?

@marvinhagemeister
Copy link
Member Author

@cromefire reactNamespace is deprecated and replaced by jsxFactory. The problem with either is that they only have an effect code-generation and not types which are checked before any code is generated.

@timnovis
Copy link

timnovis commented Dec 11, 2018

We're desperate to try out preact-compat in our TS/React project, now that createRef has been implemented in the latest update I'd love to hear if there were any decent solutions to this issue

Related: preactjs/preact-compat#357

@douggr
Copy link

douggr commented Apr 18, 2019

Sorry bringing this up once again and maybe I'm wrong, but I think JSX must remain outside preact namespace. Rewriting all JSX messy references to preact.JSX fixes the problem.

Code generated with:

douglas:~/labs
$ preact create typescript preact-x --yarn && cd preact-x

douglas:~/labs/preact-x
$ yarn add preact@500 preact-router@500

packages:

douglas:~/labs/preact-x
$ cat package.json | grep -E '^\s+"preact'
    "preact-cli": "^3.0.0-next.19",
    "preact-render-spy": "^1.3.0",
    "preact": "^10.0.0-beta.0",
    "preact-router": "^3.0.0"
douglas:~/labs/preact-x
$ 
douglas:~/labs/preact-x
$ yarn build
yarn run v1.15.2
$ preact build
Build [=================== ] 95% (2.3s) emitting
 ssr-bundle.ed2dc.css ⏤  569 B (+569 B)
        ssr-bundle.js ⏤  7.07 kB (+7.07 kB)

✖ ERROR ./node_modules/preact-render-spy/index.d.ts
ERROR in ./node_modules/preact-render-spy/index.d.ts(98,23):
TS2503: Cannot find namespace 'JSX'.
✖ ERROR ./node_modules/preact-render-spy/index.d.ts
ERROR in ./node_modules/preact-render-spy/index.d.ts(121,18):
TS2503: Cannot find namespace 'JSX'.
✖ ERROR ./node_modules/preact-render-spy/index.d.ts
ERROR in ./node_modules/preact-render-spy/index.d.ts(127,18):
TS2503: Cannot find namespace 'JSX'.
✖ ERROR ./node_modules/preact-router/index.d.ts
ERROR in ./node_modules/preact-router/index.d.ts(65,58):
TS2503: Cannot find namespace 'JSX'.
✖ ERROR ./node_modules/preact-router/match.d.ts
ERROR in ./node_modules/preact-router/match.d.ts(9,36):
TS2503: Cannot find namespace 'JSX'.
✖ ERROR ./src/components/header/index.tsx
ERROR in ./src/components/header/index.tsx(11,22):
TS2322: Type '{ children: string; activeClassName: string; href: string; }' is not assignable to type 'LinkProps'.
  Property 'children' does not exist on type 'LinkProps'.
✖ ERROR ./src/components/header/index.tsx
ERROR in ./src/components/header/index.tsx(14,22):
TS2322: Type '{ children: string; activeClassName: string; href: string; }' is not assignable to type 'LinkProps'.
  Property 'children' does not exist on type 'LinkProps'.
✖ ERROR ./src/components/header/index.tsx
ERROR in ./src/components/header/index.tsx(17,22):
TS2322: Type '{ children: string; activeClassName: string; href: string; }' is not assignable to type 'LinkProps'.
  Property 'children' does not exist on type 'LinkProps'.
✖ ERROR undefined
Done in 5.91s.

@marvinhagemeister
Copy link
Member Author

marvinhagemeister commented Apr 18, 2019

@douggr Looks like the typings for preact-render-spy need to be updated. Can you file an issue over there?

EDIT: if you're looking for enzyme-like testing capabilities we provide an officially endorsed enzyme adapter for that: https://github.com/preactjs/enzyme-adapter-preact-pure

@cromefire
Copy link

Couldn't one alias preact.JSX as JSX?

@douggr
Copy link

douggr commented Apr 18, 2019

@marvinhagemeister thank you.

I think preact-render-spy is kinda abandoned actually. But will do.

Unfortunately this issue also happens with preact-router@3.

@iainmcgin
Copy link

I encountered this issue with the type definitions for enzyme (@types/enzyme@3.9.1), which refer to JSX.Element and JSX.HTMLAttributes in a few places. I was able to hack around it by aliasing just those two types in my own type declaration file:

declare namespace JSX {
  type Element = preact.JSX.Element;
  type HTMLAttributes = preact.JSX.HTMLAttributes;
}

That was enough for me, and I'm only using Preact so I don't have the concerns of mixing React and Preact expressed as part of this issue. Does anyone know how others projects should reference JSX, so as to be agnostic to the actual provider of that type? Are there any examples from modules that work with both React and Preact X?

@ForsakenHarmony
Copy link
Member

You'll probably want to install @types/react for that

@trusktr
Copy link

trusktr commented May 11, 2020

Can someone please explain how one uses React in one file, and Preact in another file? How does TypeScript know which JSX types to use per file?

EDIT: looks like that's done with /* @jsx h */ at the top of Preact files (as long as h is imported and available in the module scope), and otherwise letting other files fall back to React's global JSX.

@marvinhagemeister
Copy link
Member Author

@trusktr The /* @jsx */ comment is the best workaround so far. The main issue is that the React JSX types are declared globally and therefore take precedence over anything else.

@trusktr
Copy link

trusktr commented May 13, 2020

It turns out the new TypeScript Project References feature is great for solving the namespace clashing. Under the premise that Preact defines JSX types globally, we can put Preact components in one folder and don't import React in any code there, and put React components in another folder and don't import Preact there, define each folder as a project reference, and finally observe the globals will be segregated on a per-project basis. Preact code will only see Preact JSX, and React code React JSX.

I've been using this technique to use multiple types of JSX components in one overall project, with jsx: preserve, and a varying compile step for JSX depending on which folder a file is in.

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

Successfully merging a pull request may close this issue.

10 participants