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

Is there a conflict between @types/node and @types/react-native? #15960

Closed
4 tasks done
bherila opened this issue Apr 18, 2017 · 52 comments
Closed
4 tasks done

Is there a conflict between @types/node and @types/react-native? #15960

bherila opened this issue Apr 18, 2017 · 52 comments

Comments

@bherila
Copy link
Contributor

bherila commented Apr 18, 2017

  • I tried using the @types/node and @types/react-native package and had problems.
  • I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
  • I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
  • Mention the authors (see Definitions by: in index.d.ts) so they can respond.

Both of the modules seem to define require and this results in some compliation trouble:

../node_modules/@types/node/index.d.ts(49,13): error TS2451: Cannot redeclare block-scoped variable 'global'.
../node_modules/@types/node/index.d.ts(73,13): error TS2300: Duplicate identifier 'require'.
../node_modules/@types/react-native/index.d.ts(8541,11): error TS2451: Cannot redeclare block-scoped variable 'global'.
../node_modules/@types/react-native/index.d.ts(8542,14): error TS2300: Duplicate identifier 'require'.

Any tips/workarounds?

@gyzerok
Copy link
Contributor

gyzerok commented Apr 19, 2017

@bherila can you, please, create a SSCCE repository which one can run by himself?

@alloy
Copy link
Collaborator

alloy commented Apr 19, 2017

It’s true, there are conflicts between the two. RN provides some features that people are used to from Node (e.g. require), however it is definitely not a full-fledged Node environment, so using the Node typings isn’t really correct.

In the end, we solved it by removing the Node typings and just sprinkle a few typings here and there where they were really needed, e.g. in our Jest setup which does run on Node: artsy/emission@25f296c#diff-0b93f41d9b1048bf2a2e50b4036a898aR2

@rawrmaan
Copy link
Contributor

rawrmaan commented Apr 19, 2017

@bherila I run this bash script whenever I do a full npm install (and in my npm postinstall script so I can deploy to Heroku). It replaces the global definitions in the RN typings with nonsense so everything compiles nicely: https://gist.github.com/rawrmaan/be47e71bd0df3f7493ead6cefd6b400c

@bherila
Copy link
Contributor Author

bherila commented Apr 20, 2017

Also, for some reason updating to the latest typings of each, and the latest typescript made the problem go away, although I still feel like it isn't "solved" per se :(

But I'll close the issue for the time being. That script also looks helpful, thanks @rawrmaan

@bherila bherila closed this as completed Apr 20, 2017
@alloy
Copy link
Collaborator

alloy commented Apr 20, 2017

Good to know 👍

@dieseldjango
Copy link

I'm running into this problem with my react-native project via a transitive dependency ing react-native-code-push, which through a chain of dependencies installs a package developed in TypeScript for node, which installs @types/node as its own dependency. I hacked around this by removing @types/node in my postinstall script. I could probably also use @rawrmaan's hack, but I'm wondering what the "right" way to deal with this would be.

Should the package that pulls in @types/node not declare it as a dependency? That would leave types in their own type definition unresolved until the consumer of the package pulls in @types/node. They could declare @types/node as a peerDependency, but then non-node consumers such as myself would get a bogus missing peer warning. Or is this a problem for the react-native-code-push package, which needs to be somehow refactored to eliminate the node dependency?

@chetmurphy
Copy link

I'm still hitting the problem with typescript 2.5.2, @types/react-native 0.48.0, and @types/node 8.0.22. I'm removing the errors by commenting out the declarations in @types/node and @types/react-native one by one (running tsc each time) starting with the top most error.

@jcampbell05
Copy link

This is a huge problem with my app which is cross platform since electron pulls it in. One solution is to structure your project in a way to have the dependencies not conflict (have electron and RN as separate packages)

Easier said than done.

@evanjmg
Copy link

evanjmg commented Nov 23, 2017

+1 - please fix this. getting with latest everything

@ou2s
Copy link

ou2s commented Nov 23, 2017

+1 I have the issue with RN 0.48.4

@jcampbell05
Copy link

Is there a way of maybe getting react native to depend on the node type definition instead of rolling it's own ?

@alloy
Copy link
Collaborator

alloy commented Nov 23, 2017

Alas, RN can’t pull in the node definitions, because RN doesn’t run inside node and so TS wouldn’t be able to tell you that certain APIs are not available.

We maintainers of the RN typings are all for accepting a solution, but please note that +1s are not going to help, the RN typings are completely driven by the needs of the maintainers and contributors.

@alloy
Copy link
Collaborator

alloy commented Nov 23, 2017

@dieseldjango Alas, the only way to currently do it is indeed to specify it as a peer dependency and leave it up to user to decide what is right for their project.

@chetmurphy
Copy link

chetmurphy commented Nov 23, 2017 via email

@evanjmg
Copy link

evanjmg commented Nov 24, 2017

@chetmurphy thanks that is definitely the cleanest workaround

@jcampbell05
Copy link

@alloy is there a way of including that workaround somehow in the typings ? i.e in documentation or just by having it in a comment at the top of the file

@alloy
Copy link
Collaborator

alloy commented Nov 24, 2017

I don’t really know if it will be read when added as a comment to the top of the file, it won’t automatically show up anywhere. People looking for this specific problem may just find this issue and maybe it’s helpful advice in general that can be added to the wiki maybe? https://github.com/DefinitelyTyped/DefinitelyTyped/wiki

cc @andy-ms

@ghost
Copy link

ghost commented Nov 28, 2017

@alloy Maybe we could add this information to the duplicate-var error message itself, since that will usually be caused by conflicting types that were automatically included. See microsoft/TypeScript#19356. CC @mhegazy

@jcampbell05
Copy link

@andy-ms that would be a great compromise for me :) and would have saved many wasted hours.

@alloy
Copy link
Collaborator

alloy commented Nov 29, 2017

@andy-ms Agreed, helpful errors are the best. I’d like to PR that, are you suggesting to add something to the error message like the following?

"Subsequent variable declarations must have the same type. Variable '{0}' has type '{1}' at {2}, but here has type '{3}'. If conflicting type definitions are being pulled in as dependencies, consider using the types compiler option to selectively enable the correct type definitions for your project."

@ghost
Copy link

ghost commented Nov 29, 2017

Should check with @mhegazy first since the first PR was reverted.

@vladinator1000
Copy link

vladinator1000 commented Dec 29, 2017

It's worth noting that @chetmurphy 's workaround needs to be added in tsconfig.json under compilerOptions like this:

{
  "compilerOptions": {
    "types": ["react", "react-native", "jest"],
    // ...
  }
} 

@corydeppen
Copy link
Contributor

Is the TS community really okay with a workaround like this? Seems a bit odd that we need to explicitly list types our projects are using after they've already been installed, being careful to make sure none are missed.

@corydeppen
Copy link
Contributor

Additionally, this workaround doesn't seem valid if your app makes use of any environment variables (e.g. process.env.REACT_APP_VAR), as you'd be relying on @types/node.

@ghost
Copy link

ghost commented Feb 10, 2018

@corydeppen

Quoting @alloy:

RN provides some features that people are used to from Node (e.g. require), however it is definitely not a full-fledged Node environment

Based on this, you shouldn't reference @types/node and should rely only on what's in @types/react-native. So if process.env is available at runtime, it should probably be added to @types/react-native.

@danwoodbury
Copy link

@kevinsperrine thanks but that dosnt seem to work for me. It fails before it gets to that point when running yarn add ... and when I run yarn install i get the error sed: can't read s/\(^declare var require: NodeRequire;\)/\/\/\1/g: No such file or directory although the file is quite clearly there as I can see it in front of my own eyes

@kevinsperrine
Copy link
Contributor

@danwoodbury Can you simply run sed -i '' "s/\(^declare var require: NodeRequire;\)/\/\/\1/g" node_modules/\@types/node/index.d.ts from the command line?

@danwoodbury
Copy link

danwoodbury commented Mar 17, 2018

nope, same error. I can manually find the line within the file it is targeting but the script cant. Very strange

@kevinsperrine
Copy link
Contributor

@danwoodbury Very. Hope you can get it straightened out. sed has different parameters depending on platform, so you might try checking into that.

@danwoodbury
Copy link

yeh odd, Will have a look into it, worst case I will just have to build the fetch commands manually until I can get this figured out. Thanks for the help

@kristerkari
Copy link
Contributor

Using @kevinsperrine's post install script works as a good workaround for me.

The only thing that you have to take into account is that there are some platform differences with sed command. So sed -i '' works nicely on OS X, but was failing for me on the linux CI server.

If the sed command fails for you, then you can use something else to do the regex replacement, or implement a workaround to support all platforms:
https://stackoverflow.com/a/38595160

@elnygren
Copy link

Consider reopening the issue, it still exists with

  "devDependencies": {
    "@types/graphql": "^0.13.4",
    "@types/node": "^10.9.2",
    "@types/react": "^16.4.12",
    "@types/react-native": "^0.56.14",
    "typescript": "^3.0.1"

I'm using Expo SDK v28.0.0 that is based on React Native 0.55.4

@esamattis
Copy link
Contributor

There's an open issue here #16825

@Kstar0722
Copy link

I got the same issue.

"react": "16.5.0",
"react-native": "^0.57.2",
"@types/react-native": "^0.46.7",
"@types/node": "^10.11.7",

@ch1ll0ut1
Copy link

Also same problem with react native and expo

@henrikra
Copy link
Contributor

Does React Native has custom versions of console and require? If not then it doesn't make sense that React Native defines them in its types

@alloy
Copy link
Collaborator

alloy commented Nov 14, 2018

@henrikra Where else should it get those definitions from? It can’t get it from the node types, because the RN environment isn’t node so that shouldn’t be loaded.

@alloy
Copy link
Collaborator

alloy commented Nov 14, 2018

@elnygren You seem to be loading the node types, that is what’s causing the problem. Please try to find a different approach.

@henrikra
Copy link
Contributor

It is common to get node types for example if you want to run your unit tests with TypeScript. In this case you need to use ts-node which includes node types

@alloy
Copy link
Collaborator

alloy commented Nov 14, 2018

I get that, we have that issue too, but the actual RN env (your production env) is not node and so that’s what should take precedence imo. In our case we added a few tiny type defs to our test suite.

@henrikra
Copy link
Contributor

What you are using to run your tests with TypeScript? I am using ts-node and it's dependency is @types/node so they are always included :/

@alloy
Copy link
Collaborator

alloy commented Nov 14, 2018

Currently we still use ts-jest https://github.com/artsy/emission/blob/f38ad50c76219943738ae901e090e4d8d9db6b69/package.json#L149-L151, but we’re in the process of switching over the entire app and tests to be just babel 7 with its TS support.

@ccorcos
Copy link

ccorcos commented Nov 22, 2018

I'm using this postinstall script to fix issues with globals.

  "scripts": {
    "postinstall": "bash ./fix-types.sh"
  },
#!/usr/bin/env bash

DIRNAME=$(dirname "$0")

# React Native declares global types that interfere with @types/node and lib dom.
rm -f $DIRNAME/node_modules/@types/react-native/globals.d.ts
sed -i '' 's|/// <reference path="globals.d.ts" />||' $DIRNAME/node_modules/@types/react-native/index.d.ts

# Namespace the globals
sed -i '' 's|declare global|declare namespace IgnoreTheseGlobals|' $DIRNAME/node_modules/@types/react-native/index.d.ts

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

No branches or pull requests