Skip to content

possible bug: Array<string> | string #13431

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

Closed
ORESoftware opened this issue Jan 12, 2017 · 14 comments · May be fixed by hixio-mh/TypeScript#5
Closed

possible bug: Array<string> | string #13431

ORESoftware opened this issue Jan 12, 2017 · 14 comments · May be fixed by hixio-mh/TypeScript#5
Labels
Needs More Info The issue still hasn't been fully clarified Question An issue which isn't directly actionable in code

Comments

@ORESoftware
Copy link

ORESoftware commented Jan 12, 2017

TypeScript Version: 2.1.4

Code

const _ = require('lodash');

export function appendFile($lines: string | Array<string>) : Observable<any> {

    let lines : Array<string> = _.flattenDeep([$lines]);

}

Expected behavior:

This should compile as far as I know.

Actual behavior:

I get this compile error:

lib/queue-proto.ts(84,9): error TS2322: Type '(string | string[])[]' is not assignable to type 'string | string[]'.
  Type '(string | string[])[]' is not assignable to type 'string[]'.
    Type 'string | string[]' is not assignable to type 'string'.
      Type 'string[]' is not assignable to type 'string'.
@RyanCavanaugh
Copy link
Member

What's the definition of flattenDeep ?

@ORESoftware
Copy link
Author

I don't know about the d.ts but it should always return an array

https://lodash.com/docs/4.17.3#flattenDeep

@k7sleeper
Copy link

Well, the definition of flattenDeep is of importance, of course.

The definition which I found is

flattenDeep<T>(array: ListOfRecursiveArraysOrValues<T>): T[]

where

interface ListOfRecursiveArraysOrValues<T> extends List<T|RecursiveArray<T>> {}
interface RecursiveArray<T> extends Array<T|RecursiveArray<T>> {}

@sandersn
Copy link
Member

What version of lodash's d.ts are you using? With 4.14, I wrote this in the test file for DefinitelyTyped':

        let $lines: string | string[];
        let lines = _.flattenDeep([$lines]);

And lines has the expected type (string | string[])[].

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Jan 12, 2017
@ORESoftware
Copy link
Author

Ok so what you guys are saying is that it should be

let $lines: string | string[];

instead of

let $lines: string | Array<string>;

I am new to TS

@sandersn
Copy link
Member

No, string[] and Array<string> are synonyms, so that shouldn't make a difference. I think the problem is that your version of lodash's d.ts is wrong. How did you obtain it? How do you reference it? Can you find its definition of flattenDeep and post it here?

If you are new to TypeScript, then Stack Overflow is a good place to ask questions for help. If you're not sure something is a bug, you might start there since the Typescript issue tracker is only for bugs and proposals.

@ORESoftware
Copy link
Author

Sure, that could be it:

// tsconfig.json

{
  "compilerOptions": {
    "types": ["node","lodash","rxjs"],
    "typeRoots": [ "node_modules/@types"],
    "compileOnSave": true,
    "target": "es2017",
    "module": "commonjs",
    "noImplicitAny": false,
    "removeComments": true,
    "preserveConstEnums": true,
    "allowJs": true,
    "allowUnreachableCode": true,
    "lib": ["es2015", "dom"]
  },
  "include": [
    "./**/*.ts"
  ],
  "exclude": [
    "node_modules"
  ]
}

//package.json

  "dependencies": {
    "colors": "^1.1.2",
    "debug": "^2.5.2",
    "live-mutex": "latest",
    "lodash": "^4.17.2",
    "replace-line": "latest",
    "rxjs": "^5.0.2",
    "uuid": "^3.0.1"
  },
  "devDependencies": {
    "@types/core-js": "^0.9.35",
    "@types/lodash": "^4.14.48",
    "@types/node": "^7.0.0",
    "async": "^2.1.4",
    "lockfile": "^1.0.3",
    "mkdirp": "^0.5.1",
    "proper-lockfile": "^2.0.0",
    "rimraf": "^2.5.4",
    "suman": "github:oresoftware/suman#dev"
  }

probably something there that's not right

@sandersn
Copy link
Member

Everything there looks ok.
Can you look in node_modules/@types/lodash/index.d.ts and find the definition of flattenDeep?

@ORESoftware
Copy link
Author

sure thing here we go

this is what it looks like:

   //_.flattenDeep
    interface LoDashStatic {
        /**
         * Recursively flattens a nested array.
         *
         * @param array The array to recursively flatten.
         * @return Returns the new flattened array.
         */
        flattenDeep<T>(array: ListOfRecursiveArraysOrValues<T>): T[];
    }

@ORESoftware
Copy link
Author

ORESoftware commented Jan 15, 2017

@sandersn any word on this bird? I don't know the answer and am still wondering why this is, given the d.ts above. thanks for your help.

@sandersn
Copy link
Member

The snippet above looks ok, and is identical to the DefinitelyTyped version that I tried with no success. I also tried exactly your code with the latest typescript compiler and couldn't reproduce the error. Can you npm install typescript@next and try with that?

My repro looked like this:

buglodash/
  index.js
  package.json
  tsconfig.json

Then I did

$ cd buglodash
$ npm install
$ tsc-latest # this is my link to the latest typescript build on my machine

The only error I got was error TS2304: Cannot find name 'Observable', so I change the return type to void and that compiled with no problems.

@sandersn sandersn added the Needs More Info The issue still hasn't been fully clarified label Jan 17, 2017
@ORESoftware
Copy link
Author

ORESoftware commented Jan 17, 2017

Yeah tried using typescript@next and I get the same errors - could I share my project with you?

if you clone this

https://github.com/ORESoftware/observable-persistent-queue

and run npm install && tsc

you should see these errors:

lib/handle-priority.ts(24,5): error TS2322: Type '{}' is not assignable to type 'IPriorityInternal'.
  Property 'first' is missing in type '{}'.
lib/helpers.ts(203,9): error TS2322: Type '(string | string[])[]' is not assignable to type 'string[]'.
  Type 'string | string[]' is not assignable to type 'string'.
    Type 'string[]' is not assignable to type 'string'.
lib/queue-proto.ts(86,9): error TS2322: Type '(string | string[])[]' is not assignable to type 'string | string[]'.
  Type '(string | string[])[]' is not assignable to type 'string[]'.
    Type 'string | string[]' is not assignable to type 'string'.
      Type 'string[]' is not assignable to type 'string'.
lib/queue.ts(396,18): error TS2339: Property 'connect' does not exist on type 'Observable<{}>'.
lib/sed.ts(26,11): error TS2322: Type '(string | string[])[]' is not assignable to type 'string[]'.
  Type 'string | string[]' is not assignable to type 'string'.
    Type 'string[]' is not assignable to type 'string'.

2 of those errors relate to the question at hand, the codebase is pretty small and you should be able to use the tsc error trace to see where the .ts source in question is.

@mhegazy mhegazy closed this as completed Apr 21, 2017
@thorn0
Copy link

thorn0 commented Jul 17, 2017

@sandersn

What version of lodash's d.ts are you using? With 4.14, I wrote this in the test file for DefinitelyTyped':

   let $lines: string | string[];
   let lines = _.flattenDeep([$lines]);

And lines has the expected type (string | string[])[].

The expected type should have been string[].

I tried the latest version 4.14.63 of @types/lodash with TS 2.4.2-insiders.20170630.

import * as _ from 'lodash';
let a = _.flattenDeep([1,2,[3]]);

a should be of type number[], but it's (number | number[])[].

So these typings are definitely wrong. Please reopen the issue. Although I'm not sure whether TS's type system is capable enough to fix this.

@sandersn
Copy link
Member

sandersn commented Aug 2, 2017

@thorn0 this is indeed a limitation of the Typescript compiler. It doesn't like infinite types (most type systems don't!). The incorrect behaviour is what you get when it infers to an infinite type. If lodash used type alias syntax instead of interface RecursiveArray<T> extends Array<T|RecursiveArray<T>> {}, you'd see the error upfront instead of getting a weird error later.

The best fix that I can think of is to give flatten, flattenDeep and flattenDepth the same type: function flatten<T>(xs: T[][]): T[]. This is how flow behaves given the current lodash definitions.

This is a limitation of the type system that is unlikely to be fixed, so the correct workaround would be to open a PR on DefinitelyTyped. I'll leave this issue closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs More Info The issue still hasn't been fully clarified Question An issue which isn't directly actionable in code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants