React-hot-loader: It references "react" in the public typing declarations but doesn't add a dependency for "@types/react"

[slavafomin]

Hello!

Thank you for this great plugin!

However, in published index.d.ts file you are referencing the react package in some exports of yours. However, you don't specify the dependency for @types/react, which causes issues in projects where TypeScript compiler is running in "strict" mode and where dependencies are installed by a more strict package manager like pnpm (Rush).

It gives the following TypeScript error during application compilation:

ERROR in ~/app/common/temp/node_modules/.registry.npmjs.org/react-hot-loader/[email protected][email protected]/node_modules/react-hot-loader/index.d.ts(1,24):

TS7016: Could not find a declaration file for module 'react'.
'~/app/common/temp/node_modules/.registry.npmjs.org/react/16.10.2/node_modules/react/index.js' implicitly has an 'any' type.
If the 'react' package actually exposes this module, consider sending a pull request to amend 'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react`

As you have react in your peer dependencies, you should also add @types/react there as well in order for TypeScript projects to be able to resolve typing declarations correctly.

Thanks!

[theKashey]

That's bad. That means that I have to change peer dependencies for all 100500 packages I've ever created.

Is there any pnpm issue to read something about required changes (and ways to test them), as well as "script" TS mode?

[theajr]

@slavafomin Can you please provide the steps to reproduce?

[slavafomin]

@theajr I can't give you a detailed example, but the gist of it is that you need to create a TypeScript React application using pnpm instead of npm (it handles dependencies in a more strict and correct manner), then replace react-dom with implementation from this package using Webpack aliases. Also, the TypeScript should be set to a "strict" mode. I'm using ForkTsCheckerWebpackPlugin to perform typing checks and Babel to transpile from TypeScript.

[slavafomin]

@theKashey

That means that I have to change peer dependencies for all 100500 packages I've ever created.

You only need to specify @types/... dependencies as a non-dev dependencies if you are exposing them in your TypeScript declarations. This is a rare case, actually, and it's very easy to miss.

[slavafomin]

For those, who are using Rush the following workaround could be used to mitigate the situation until it's fixed in this package:

// common/config/rush/pnpmfile.js

'use strict';

const rewriteRules = {
  // @todo: remove this rule when the issue is fixed
  // @url https://github.com/gaearon/react-hot-loader/issues/1359
  'react-hot-loader': {
    setDependencies: {
      '@types/react': '>=16',
    },
  },
};

// https://rushjs.io/pages/configs/pnpmfile_js/

module.exports = {
  hooks: {
    readPackage: rewriteManifest,
  },
};


function rewriteManifest(manifest, context) {

  normalizeManifest(manifest);

  const { name, dependencies } = manifest;

  const rule = rewriteRules[name];
  if (rule) {
    Object.assign(dependencies, rule.setDependencies);
    context.log(`Rewritten package "${name}" dependencies`);
  }

  return manifest;

}

function normalizeManifest(manifest) {
  if (!manifest.dependencies) {
    manifest.dependencies = {};
  }
}

Feel free to add other dependencies in the rewriteRules config if you need it ;)

[theKashey]

Fixed in 4.12.16

[theKashey]

@slavafomin - would it work if @types/react would be a peerDependency?

[slavafomin]

@theKashey yes, I would recommend to add it as a peer dependency.

[SleeplessByte]

It should be added _both_ as a peerDependency and devDependency and not as a dependency (re: #1391)

[theKashey]

4.12.17 released with a fix

[SleeplessByte]

Works!