fkhadra / react-toastify

React notification made easy 🚀 !
https://fkhadra.github.io/react-toastify/introduction
MIT License
12.58k stars 692 forks source link

2.1.3 breaks TypeScript build #63

Closed thchia closed 6 years ago

thchia commented 6 years ago
ERROR in [at-loader] ./node_modules/react-toastify/index.d.ts:23:16 
    TS2304: Cannot find name 'Transition'.
ERROR in [at-loader] ./node_modules/react-toastify/index.d.ts:37:37 
    TS2686: 'React' refers to a UMD global, but the current file is a module. Consider adding an import instead.

I think there need to be imports for React and Transition.

LeopoldLerch commented 6 years ago

same here

thchia commented 6 years ago

I tried to do a quick fix, but I ran into problems because (I think) the type declaration file for react-transition-group specifies a wildcard dependency for React. I am still on React v15.x so there's a collision with the latest (v16.x) version.

fkhadra commented 6 years ago

How can I validate my definition file? I'm a beginner with typescript

thchia commented 6 years ago

The gist of it is you need to

import React from 'react'
import { Transition } from 'react-transition-group'

And make sure that both of these type files are listed as dependencies (@types/react @types/react-transition-group).

The problem I had was that although this fixes this package, I got errors because @types/react-transition-group referenced @types/react: "*", which caused the types for v16 to be included in my project when I am using v15.x still.

fkhadra commented 6 years ago

Ok thanks.

But I think @types/react @types/react-transition-group need to be listed as devDependencies instead of dependencies.

When I look at other packages most of the time @types/* is listed as DevDependencies.

thchia commented 6 years ago

Sorry yes I meant as devDependencies.

fkhadra commented 6 years ago

Thanks for your help !

fkhadra commented 6 years ago

Just pushed a new version. Can you tell me if it fix the issue please.

Thank you.

LeopoldLerch commented 6 years ago

last bug fuxed, but new error occuring now:

ERROR in [at-loader] ./node_modules/react-toastify/index.d.ts:1:8 TS1192: Module '"C:/Projekte/DAP/DAP/Webkomponenten/node_modules/(at)types/react/index"' has no default export.

fkhadra commented 6 years ago

Really weird.

thchia commented 6 years ago
ERROR in [at-loader] ./node_modules/react-toastify/index.d.ts:2:24
    TS7016: Could not find a declaration file for module 'react-transition-group/Transition'. '/Users/thomaschia/argomi/node_modules/react-transition-group/Transition.js' implicitly has an 'any' type.
  Try `npm install @types/react-transition-group/Transition` if it exists or add a new declaration (.d.ts) file containing `declare module 'react-transition-group/Transition';`

I think this is because @types/react-transition-group is a devDependency. It doesn't get included when installing this library.

-- EDIT Actually that doesn't make sense. Non-TS users will get it too if it is specified in dependencies.

fkhadra commented 6 years ago

Hmm. I'm wondering why it's not happening for React.

-- EDIT So I must leave it as devDependencies, right ?

thchia commented 6 years ago

That's because I already have the type file for react. I do not have it for the transition library.

fkhadra commented 6 years ago

I'm wondering if it's not due to a cache issue

LeopoldLerch commented 6 years ago

no, because react.index.ts doesn´t have a default export. So you have to import it either using "*" or single components in {}

fkhadra commented 6 years ago

Maybe I need to turn import React from "react" to import React = require("react")

LeopoldLerch commented 6 years ago

or

import * as React from "react"

fkhadra commented 6 years ago

Can you check if it works for you when doing import * as React from "react" inside the node_modules/react-toastify/index.d.ts.

fkhadra commented 6 years ago

I saw in definitlyTypes that `import React = require('React') is used

LeopoldLerch commented 6 years ago

import * ....

works

require might work too. but i think it is recommended to avoid require if possible

fkhadra commented 6 years ago

Sure. Thank you for the test. I fix it now.

fkhadra commented 6 years ago

Fix pushed. Anything is ok for you now ?

LeopoldLerch commented 6 years ago

works now @me