coatue-oss/react2angular

Move the @types dependencies to dev-dependencies?

tyronedougherty opened this issue · 4 comments

Currently there are two dependencies for @types definitions that should be in the devDependencies instead:

"dependencies": {
    "@types/angular": ">=1.5",
    "@types/lodash.frompairs": "^4.0.3",
    "angular": ">=1.5",
    "lodash.frompairs": "^4.0.1",
    "ngcomponent": "^4.1.0"
  }

The reason this is an issue is because if you are already using @types/angular (or @types/lodash.frompairs for that matter) in your project, after adding react2angular the two type definitions will clash, resulting in the following error:

Failed to compile.

[at-loader] ./node_modules/react2angular/node_modules/@types/angular/jqlite.d.ts:29:5 
    TS2375: Duplicate number index signature.

I therefore propose moving those two type definitions to dev dependencies instead of regular dependencies. Thoughts?

Hi @tyronedougherty! There are a few things here:

  1. Why are you requiring two versions of the types? The versions required by react2angular are really permissive on purpose, to maximize compatibility with existing projects. If you upgrade to the latest @types/ versions and you're still getting conflicts, delete your package-lock.json/yarn-lock.json and run npm install/yarn install again.
  2. Have you tried explicitly excluding ./node_modules/react2angular/node_modules/@types in your tsconfig.json?

If (1) doesn't work, please report back and use (2) as a workaround in the meantime. The fix would be to move all the @types/ to peerDependencies (because they are required to consume react2angular in a TypeScript project).

I'm actually using Flow in my project (crazy, I know). But yeah it seemed odd to have to install types for a type system I'm not actually using.

@SethDavenport You're right, and I couldn't agree more. Unfortunately there's no good story around this today. There's no such thing as dependenciesForTypeScriptProjectsOnly.

One way to approximate it is to put all the @types/ in peerDependencies, which has the pro of reducing install size for non-TS users (like you), and the cons of an annoying warning every time non-TS users run npm i and extra complexity of having to manually install these for TS users.

By the way, if you want to add Flow typings to this package, we would more than appreciate it :)

@bcherny thanks so much for your quick response! Turns out deleting my yarn.lock file did the trick after all 😄

Many thanks! I'll close this now.