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

fix(types): give explicit type to empty arrays #7557

Merged
merged 3 commits into from
Jun 18, 2019
Merged

Conversation

kevinmarrec
Copy link
Contributor

@kevinmarrec kevinmarrec commented Jun 18, 2019

Description

Fix types of some places where empty arrays with implicit type would resolve to never[] type instead of any[] and cause TypeScript errors.

Motivation and Context

Running a simple TypeScript project with Vuetify was displaying 5 errors regarding Vuetify types, see below the screenshots.

Screenshots of errors

Weirdly, in editor, we can't see these errors in Vuetify source code but only inside a TS project using Vuetify under node_modules/vuetify/src/util/{file}.ts (it's probably due to some tsconfig.json options).

In TypeScript, empty arrays that have no explicit types are resolving to the type never[], so pushing anything (even of any type) in to this array leads to a type error "Argument of type 'any' is not assignable to parameter of type 'never'."

It can be easily fixed by explicitly defining the array type as any[].

Sources :
microsoft/TypeScript#9976
microsoft/TypeScript#9976 (comment)

How Has This Been Tested?

Locally in the project I had the 5 errors (seen on screenshots). These changes fixed all of them.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and breaking changes).
  • My code follows the code style of this project.

@kevinmarrec kevinmarrec requested a review from a team June 18, 2019 08:50
@codecov
Copy link

codecov bot commented Jun 18, 2019

Codecov Report

Merging #7557 into next will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #7557   +/-   ##
=======================================
  Coverage   86.94%   86.94%           
=======================================
  Files         325      325           
  Lines        8536     8536           
  Branches     2153     2153           
=======================================
  Hits         7422     7422           
  Misses       1024     1024           
  Partials       90       90
Impacted Files Coverage Δ
packages/vuetify/src/util/console.ts 68.33% <100%> (ø) ⬆️
packages/vuetify/src/util/helpers.ts 92% <100%> (ø) ⬆️

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 3bdffae...3ecd9b3. Read the comment docs.

Co-Authored-By: Kael <kaelwd@gmail.com>
Copy link
Contributor

@dsseng dsseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dsseng dsseng merged commit 121c694 into next Jun 18, 2019
@dsseng dsseng deleted the fix/types-never-arrays branch June 18, 2019 14:13
@lock lock bot locked as resolved and limited conversation to collaborators Jul 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants