Skip to content
This repository was archived by the owner on Jul 30, 2020. It is now read-only.

Fix local/import type declaration conflict #79

Merged
merged 2 commits into from
Nov 13, 2019

Conversation

aiham
Copy link
Contributor

@aiham aiham commented Nov 7, 2019

What:

Avoid importing a type with the same name as a local type.

Why:

TypeScript 3.7 introduced a breaking change which fails when you declare a local type with the same name as an imported type: https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#local-and-imported-type-declarations-now-conflict

node_modules/@testing-library/react-native/typings/query-helpers.d.ts:1:10 - error TS2440: Import declaration conflicts with local declaration of 'ReactTestInstance'.

1 import { ReactTestInstance } from 'react-test-renderer';

How:

Use an import alias and do proper module augmentation.

Checklist:

  • Documentation added to the
    docs site (N/A)
  • Typescript definitions updated
  • Tests (N/A)
  • Ready to be merged

Fixes an error introduced by a backwards incompatible change in
TypeScript 3.7 which fails if you declare a local type with the same
name as an imported type:
https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#local-and-imported-type-declarations-now-conflict
@codecov
Copy link

codecov bot commented Nov 7, 2019

Codecov Report

Merging #79 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #79   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          20     20           
  Lines         268    268           
  Branches       67     67           
=====================================
  Hits          268    268

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2097cc7...f9b8626. Read the comment docs.

@aiham
Copy link
Contributor Author

aiham commented Nov 7, 2019

I'm not sure if proxyElement() expects a node argument with the getProp and parentNode keys or if they are only meant to be part of NativeTestInstance. The current change adds them to both, let me know if that's not the intention and I'll change it to only add to NativeTestInstance.

Yeah I think getProp/parentNode only belong on NativeTestInstance so I've updated the PR.

@bcarroll22
Copy link
Collaborator

Thanks for this! Really appreciate it!

@allcontributors please add @aiham for code.

@allcontributors
Copy link
Contributor

@bcarroll22

I've put up a pull request to add @aiham! 🎉

@bcarroll22 bcarroll22 merged commit c72f45f into testing-library:master Nov 13, 2019
@bcarroll22
Copy link
Collaborator

🎉 This PR is included in version 5.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aiham aiham deleted the fix/support-typescript-3.7 branch November 14, 2019 00:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants