React-hooks-testing-library: Duplicated identifier Errors with TypeScript

[fxOne]

• react-hooks-testing-library version: 1.0.4
• react-test-renderer version: 16.8.6
• react version: 16.8.17
• @types/react version: 16.8.17
• typescript version: 3.3.3333
• node version: 11.9.0
• npm version: 6.7.0

Relevant code or config:

-

What you did:

I installed the latest Version of react-hooks-testing-library and got after a typecheck the following error.

What happened:

node_modules/@testing-library/react-hooks/node_modules/@types/react/index.d.ts:2817:14 - error TS2300: Duplicate identifier 'LibraryManagedAttributes'.

2817         type LibraryManagedAttributes<C, P> = C extends React.MemoExoticComponent<infer T> | React.LazyExoticComponent<infer T>
                  ~~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/@types/react/index.d.ts:2816:14
    2816         type LibraryManagedAttributes<C, P> = C extends React.MemoExoticComponent<infer T> | React.LazyExoticComponent<infer T>
                      ~~~~~~~~~~~~~~~~~~~~~~~~
    'LibraryManagedAttributes' was also declared here.

node_modules/@testing-library/react-hooks/node_modules/@types/react/index.d.ts:2881:13 - error TS2717: Subsequent property declarations must have the same type.  Property 'iframe' must be of type 'DetailedHTMLProps<IframeHTMLAttributes<HTMLIFrameElement>, HTMLIFrameElement>', but here has type 'DetailedHTMLProps<IframeHTMLAttributes<HTMLIFrameElement>, HTMLIFrameElement>'.

2881             iframe: React.DetailedHTMLProps<React.IframeHTMLAttributes<HTMLIFrameElement>, HTMLIFrameElement>;
                 ~~~~~~

node_modules/@types/react/index.d.ts:2816:14 - error TS2300: Duplicate identifier 'LibraryManagedAttributes'.

2816         type LibraryManagedAttributes<C, P> = C extends React.MemoExoticComponent<infer T> | React.LazyExoticComponent<infer T>
                  ~~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/@testing-library/react-hooks/node_modules/@types/react/index.d.ts:2817:14
    2817         type LibraryManagedAttributes<C, P> = C extends React.MemoExoticComponent<infer T> | React.LazyExoticComponent<infer T>
                      ~~~~~~~~~~~~~~~~~~~~~~~~
    'LibraryManagedAttributes' was also declared here.

Reproduction:

Problem description:

The issue is that I use the @types/react: 16.8.17 while this library uses @types/react: 16.8.22 which results in 2 different installed @types/react versions. After updating to the latest @types/react the problem was solved.

Suggested solution:

There should be any typescript dependency installen when I install this package.

[mpeyper]

We briefly had s release where we published a version that had a pinned version of the @types/react dependency, but we fixed that in 1.0.4 so I'm not sure why, assuming you also use a semver range, that you would end up with a duplicate.

None-the-less, as part of our package, we require the react type definitions, so we need to include it as a dependency. I'm not sure what the alternatives are that could avoid this circumstance from occurring? Happy to hear suggestions.

[fxOne]

I think the problem should be solve by moving the @types definitions from the dependencies to the devDependencies in the package.json.

See also: https://docs.npmjs.com/files/package.json#devdependencies

[mpeyper]

If my type definitions have a dependency on another packages type definitions, then should they not be a dependency of my package?

Genuinely asking as I'm not a typescript use myself.

I can see that they could be considered a peer dependency, similar to react itself, but this is troublesome for non-typescript users as they will get a peer dependency if they don't install it, even though they don't need it. Perhaps this is a case for optional dependencies?

[fxOne]

I'm also new to TypeScript but when i look around I can only find projects which have their @types ind the devDependencies e.g.: https://github.com/jaredpalmer/formik/blob/4e629c9938ec628452af2ff2c2d537b795de1f56/package.json#L59

When I use TypeScript I already have the @types installed as I need them for development and when I don't use TypeScript I dont want to have the packges installed at all.

[mpeyper]

Sorry for the delayed response.

You would only have the dependency installed if it was a library you were already using (which is likely in this case, but potentially not for all libraries out there). Assuming the depenency is a a compatible sevmer range, it the npm install should resolve to the same instance. If they are not compatibile, then you would have a problem anyway, even if it was a peerDependency.

Conversely, the cost of installing some type dependencies when you don't want them is pretty small.

[fxOne]

After doing some more research I found this ticket which says that it is the correct way to use dependency to add the @types: https://github.com/microsoft/types-publisher/issues/81

At the end there are also some people which have compatibility issues when the @types are added to dependency so that they add them to devDependency.

Is it possible to change the "@types/react": "^16.8.22" dependency to "@types/react": ">=16.8.0" as this was the version when react hooks was introduced and we can use this great testing library?

[mpeyper]

From my understanding of semver ranges, ">=16.8.0" would match any newer version, whereas "^16.8.0" would only match 16.x.x versions, which is probably safer to prevent breaking changes biting our users.

I use a bot for keeping dependency versions up to date, so I'm not sure if that will roll through a push it back up to latest again if we open up the range a bit more.

[fxOne]

As far as I can see you can configure your bot to exclude dependencies: enable
Also you more configuration options with the packageRules.

[tcoldmf]

I have the same problem.

[mpeyper]

I have updated the version range in version 2 to be ">=16.9.0" with the intention if leaving at the minimum version for async act calls (which we now rely on so it can't be ">=16.8.0" like suggested above.

I'll close this now, but please let me know if this is still causing issues so we can try again at fixing it.

[filame]

I use version 2.0.1 and the issue still appears.

[atomicpages]

@mpeyper same issue.

Using: @testing-library/[email protected]

npm ls @types/react

[email protected]
├─┬ @testing-library/[email protected]
│ ├── @types/[email protected] 
│ └─┬ @types/[email protected]
│   └── @types/[email protected]  deduped
├── @types/[email protected] 
└─┬ @types/[email protected]
  └── @types/[email protected]  deduped

Maybe add it as an optional dependency?

[mpeyper]

Clearly there is more to be done here.

@atomicpages (or anyone), are you able to run npm dedupe and see if that helps?

[atomicpages]

Running dedupe works (how I got around it in the first place 😉). But it is a pretty bad side effect, not 100% sure how this can be resolved permanently.

[mpeyper]

For anyone following this issue, I'm thinking we're going to follow the path of some other testing-library packages and move the types to DefinitelyTyped and make them a dependency here.

To be honest, I don't know how it is any different from what we're currently doing (other than moving the dependencies in a layer) or that it will make this situation any better, but I'm not seeing any complaints on the approach in the other package repos.

Happy for anyone to take the reigns on either the DefinatelyTyped side or the relevant changes here.

[mpeyper]

For anyone following this, I've created the PR to add the type definitions to DefinitelyTyped -> https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38715

[mpeyper]

The DefinitelyTyped changes went out in v2.0.2. Please update and let me know if there are still any issues.

[felixfbecker]

Could you document this as a breaking change in the 2.0.2 patch notes?

[mpeyper]

Apologies about missing the release notes (I forgot to push the tags so didn't have that as a trigger to update them). I've updated them now.

I went with a non-breaking change version bump as the type definitions have not changed, just moved and it is hopefully resolving an issue some people are seeing on the current version. For anyone not having this issue, there should be no changes required.