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

Typescript: definitions overhaul and parity with React #639

Closed
Hotell opened this Issue Apr 11, 2017 · 21 comments

Comments

Projects
None yet
7 participants
@Hotell
Copy link

Hotell commented Apr 11, 2017

Current state of preact type definitions is not in very good shape and is impossible to use it with any React library.

Problems:

  • TS strict mode compliance missing
  • old TS ambient types module format
  • mixed definitions for preact/devtools module that are doing anything
  • impossible to use with React libraries
  • very few compiler tests for real life Preact/React patterns which are used in daily world
  • no Standardization/Rules how to write/maintain type definitions

What needs to be done:

  • use new TS format ( >TS 2.0 )
  • align with React definitions
  • make definitions pluggable so when preact/compat is used, TS definitions for React libraries will just work ( same for preact/devtools etc )
  • tweak JSX definitions to be on par with React but also leverage DOM as we do in Skate ( because PReact is also closer to the metal 🤘 )

We are using Preact, Skate with Typescript at work ( large enterprise company ), and have to override everything manually, so everything works. With that said I can contribute to this project by setting up everything how it needs to be. Also we relly on Preact in upcoming SkateJS 5, and this is kind of blocker for us.

Hope this makes things clearer.

@developit

This comment has been minimized.

Copy link
Owner

developit commented Apr 15, 2017

I'm all for this. Anything you think we can do to make that PR easier to dissect?

@screendriver

This comment has been minimized.

Copy link

screendriver commented Apr 25, 2017

Does this issue and the PR #621 also fix following issue? I'm using preact and write my tests with enzyme (thankfully preact-compat-enzyme exists!). Unfortunately I get a lot of these errors as long as I add the @types/enzyme type definitions because they depend on @types/react:

node_modules/@types/react/index.d.ts(2727,47): error TS2403: Subsequent variable declarations must have the same type.  Variable 'props' must be of type 'any', but here has type '{}'.

At the moment the only solution that I know is not to add @types/enzyme and make an own declaration declare module 'enzyme'; and have to work with any in my tests 😕

@Hotell

This comment has been minimized.

Copy link
Author

Hotell commented Apr 25, 2017

Yes it solves everything. As I've said we are using custom type defs for preact and preact-compat at work, otherwise it won't work with react router, react forms, i18n-next, enzyme and so on in matter of compile time errors

@Hotell

This comment has been minimized.

Copy link
Author

Hotell commented Apr 25, 2017

I'm currently on Vacation, sorry for radio silence. I'll be back around May 8th so yeah, thx for patience ☮️✌️

@developit

This comment has been minimized.

Copy link
Owner

developit commented Apr 26, 2017

no problem man 💃
hope you're somewhere warm haha

@Hotell

This comment has been minimized.

Copy link
Author

Hotell commented Apr 26, 2017

Haha yup, in Miami ;) :P

@screendriver

This comment has been minimized.

Copy link

screendriver commented Apr 26, 2017

Cool! I'm really looking forward for this.

One little question:

As I've said we are using custom type defs for preact and preact-compat

How do you do that? Is there a TypeScript compiler option that says "ignore the shipped .d.ts file from preact and use my own declarations file instead?" I was always searching for that and didn't find anything. The only thing that I know is simply not to add a @types/yourlibrary dependency and write my own for that. But for libraries that already ship with type declarations (like preact)? 🤔

By the way: I wish you a nice and sunny vacation 😎 ☀️

@pspeter3

This comment has been minimized.

Copy link

pspeter3 commented May 20, 2017

Is there anything I can do to help with this?

@developit

This comment has been minimized.

Copy link
Owner

developit commented May 20, 2017

PR reviews would be a huge help here. There's so much value, but it's also a big change to wade through cause @Hotell works too fast 😜

@pspeter3

This comment has been minimized.

Copy link

pspeter3 commented May 20, 2017

Is #621 the canonical one? Also is there a goal of ever converting preact to TypeScript?

@developit

This comment has been minimized.

Copy link
Owner

developit commented May 21, 2017

Converting preact itself to typescript is very very unlikely, since it would be a bit infeasible to produce output that small. Preact is basically written in ES5, just with let and const.

@Hotell

This comment has been minimized.

Copy link
Author

Hotell commented May 21, 2017

Converting preact itself to typescript is very very unlikely, since it would be a bit infeasible to produce output that small.

Why do you think that ? :) Typesript is just Javascript, so you get same output as with vanilla JS, no payload increase

Preact is basically written in ES5, just with let and const.

Yup and TS is again just JS, you can use only those Ecmascript features, which you need

Overall, if Preact wont use typed JS, I very highly recommend using Typescript in the background, because it is capable of checking your code without writing any TypeScript at all. #powerOverwhelming

https://twitter.com/martin_hotell/status/865982432763404288

@screendriver

This comment has been minimized.

Copy link

screendriver commented May 22, 2017

You can also read about it here (just search for Errors in .js files with --checkJs). Just add checkJs to your tsconfig.json and you've got type check support "magically" in JS files 😎

@qwertie

This comment has been minimized.

Copy link

qwertie commented Apr 28, 2018

I noticed that Preact and @types/react cannot be installed simultaneously. I assume this issue is the reason?

Repro steps: 1. create empty folder with tsconfig.json:

{"compilerOptions":{
    "target": "es5",
    "module": "umd",
    "moduleResolution": "node",
    "sourceMap": true,
    "jsx": "react",
}}
  1. Creata app.tsx:
/** @jsx preact.h */
import * as preact from 'preact';

preact.render(
  <h1>Hello, world!</h1>,
  document.getElementById('app')
);
  1. Run these commands (they all work fine):

    npm init -y
    npm install -g typescript
    npm install preact
    tsc

But with one more command...

npm install --save-dev @types/react

tsc goes insane:

app.tsx(5,3): error TS2559: Type '{ children: string; }' has no properties in common with type 'HTMLAttributes'.
node_modules/@types/react/index.d.ts(2239,47): error TS2717: Subsequent property declarations must have the same type.  Property 'props' must be of type 'any', but here has type '{}'.
node_modules/@types/react/index.d.ts(2249,13): error TS2717: Subsequent property declarations must have the same type.  Property 'a' must be of type 'HTMLAttributes', but here has type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.
node_modules/@types/react/index.d.ts(2250,13): error TS2717: Subsequent property declarations must have the same type.  Property 'abbr' must be of type 'HTMLAttributes', but here has type 'DetailedHTMLProps<HTMLAttributes<HTMLElement>, HTMLElement>'.
node_modules/@types/react/index.d.ts(2251,13): error TS2717: Subsequent property declarations must have the same type.  Property 'address' must be of type 'HTMLAttributes', but here has type 'DetailedHTMLProps<HTMLAttributes<HTMLElement>, HTMLElement>'.
node_modules/@types/react/index.d.ts(2252,13): error TS2717: Subsequent property declarations must have the same type.  Property 'area' must be of type 'HTMLAttributes', but here has type 'DetailedHTMLProps<AreaHTMLAttributes<HTMLAreaElement>, HTMLAreaElement>'.

*** ABOUT 150 MORE ERRORS ***

The errors start at line 2239 in @types/react when it tries to define JSX (declare global { namespace JSX { ... ).

@marvinhagemeister

This comment has been minimized.

Copy link
Collaborator

marvinhagemeister commented Apr 28, 2018

@qwertie No, this is a separate issue, see #1036. The reason they are clashing is that both define the JSX types globally. The recent TypeScript 2.8 release added support for per-file JSX types, but neither @types/react nor we do support that for now. We definitely intend to add support for that in the future though.

@marvinhagemeister

This comment has been minimized.

Copy link
Collaborator

marvinhagemeister commented Apr 28, 2018

Closing this as the typings have been completely rewritten in 8.2.8

@theKashey

This comment has been minimized.

Copy link

theKashey commented May 7, 2018

Using Preact 8.2.9.
Seeing all those messages.

@marvinhagemeister

This comment has been minimized.

Copy link
Collaborator

marvinhagemeister commented May 7, 2018

@theKashey What kind of messages? That@types/react cannot be installed along preact or something else from this thread? The former should be tracked here #1036

@theKashey

This comment has been minimized.

Copy link

theKashey commented May 7, 2018

Everything is about TS2717, and yes - I am mixing React and Preact - that is the problem.

@Hotell

This comment has been minimized.

Copy link
Author

Hotell commented May 7, 2018

@theKashey I've created this a while ago, so Preact is usable with Typescript and React packages without issues. You may find it useful https://github.com/Hotell/react-preact-typescript-interop

@marvinhagemeister

This comment has been minimized.

Copy link
Collaborator

marvinhagemeister commented May 7, 2018

@Hotell You rock, that is really awesome 🎉 Would you be interested in pushing some of your changes/learnings upstream?

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