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

Object spread with rest arguments causes compile error #12200

Closed
sasxa opened this issue Nov 12, 2016 · 11 comments
Closed

Object spread with rest arguments causes compile error #12200

sasxa opened this issue Nov 12, 2016 · 11 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@sasxa
Copy link

sasxa commented Nov 12, 2016

TypeScript Version: 2.2.0-dev.20161112

Code

// A *self-contained* demonstration of the problem follows...
export class ToggleTests {
  state: any = {};

  toggle1(key: string) {
    console.log({ ...this.state, [key]: !this.state[key] });
  }

  toggle2(...keys: string[]) {
    console.log({ ...this.state, [keys[0]]: !this.state[keys[0]] });
  }

  toggle3(...keys: string[]) {
    console.log(keys.reduce((dict, key) => ({ ...dict, [key]: !this.state[key] }), {}));
  }

  toggle4(...keys: string[]) {
    console.log(keys.reduce((dict, key) => {
      dict[key] = !this.state[key];
      return dict;
    }, {}));
  }
}

Expected behavior:
It should work (;
Actual behavior:
This code won't compile. If I delete/comment toggle2() and toggle3() methods it will compile and toggle1() and toggle4() methods work as expected.

Here's full error stack:

TypeError: Cannot read property 'kind' of undefined
    at isNumericName (~\node_modules\typescript\lib\typescript.js:33945:24)
    at getObjectLiteralIndexInfo (~\node_modules\typescript\lib\typescript.js:34000:48)
    at createObjectLiteralType (~\node_modules\typescript\lib\typescript.js:34137:67)
    at checkObjectLiteral (~\node_modules\typescript\lib\typescript.js:34128:52)
    at checkExpressionWorker (~\node_modules\typescript\lib\typescript.js:37309:28)
    at checkExpression (~\node_modules\typescript\lib\typescript.js:37268:42)
    at checkExpressionWorker (~\node_modules\typescript\lib\typescript.js:37320:28)
    at checkExpression (~\node_modules\typescript\lib\typescript.js:37268:42)
    at checkExpressionCached (~\node_modules\typescript\lib\typescript.js:37184:38)
    at getReturnTypeFromBody (~\node_modules\typescript\lib\typescript.js:36190:24)
    at getReturnTypeOfSignature (~\node_modules\typescript\lib\typescript.js:28554:28)
    at checkFunctionExpressionOrObjectLiteralMethodDeferred (~\node_modules\typescript\lib\typescript.js:36439:21)
    at checkDeferredNodes (~\node_modules\typescript\lib\typescript.js:40914:25)
    at checkSourceFileWorker (~\node_modules\typescript\lib\typescript.js:40948:17)
    at checkSourceFile (~\node_modules\typescript\lib\typescript.js:40928:13)
    at Object.forEach (~\node_modules\typescript\lib\typescript.js:1276:30)

http://plnkr.co/edit/NIz1MhKpK28Afyf5fsbx?p=preview

@DanielRosenwasser
Copy link
Member

Hm, I'm not able to repro this - what set of options are you using, and how are you compiling it?

@sasxa
Copy link
Author

sasxa commented Nov 13, 2016

I'm using a gulp-typescript to compile my tsconfig.json is:

{
  "compilerOptions": {
    "baseUrl": "./src",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "isolatedModules": true,
    "lib": [ "dom", "es2015", "es2017" ],
    "module": "system",
    "moduleResolution": "node",
    "sourceMap": true,
    "target": "es5"
  }
}

I'm getting the same error in this plunker if I uncomment toggle2() or toggle3() methods...

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2016

I am not getting this either.

c:\test\12200>ls
[.]             [..]            app.ts          main.ts         [node_modules]  tsconfig.json

c:\test\12200>type tsconfig.json
{
  "compilerOptions": {
    "baseUrl": "./src",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "isolatedModules": true,
    "lib": [ "dom", "es2015", "es2017" ],
    "module": "system",
    "moduleResolution": "node",
    "sourceMap": true,
    "target": "es5"
  }
}

c:\test\12200>type main.ts
//main entry point
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import {AppModule} from './app';

platformBrowserDynamic().bootstrapModule(AppModule)

c:\test\12200>type app.ts
//our root app component
import {Component, NgModule} from '@angular/core'
import {BrowserModule} from '@angular/platform-browser'

export class ToggleTests {
  state: any = {};

  toggle1(key: string) {
    console.log({ ...this.state, [key]: !this.state[key] });
  }

  toggle2(...keys: string[]) {
    console.log({ ...this.state, [keys[0]]: !this.state[keys[0]] });
  }

  toggle3(...keys: string[]) {
    console.log(keys.reduce((dict, key) => ({ ...dict, [key]: !this.state[key] }), {}));
  }

  toggle4(...keys: string[]) {
    console.log(keys.reduce((dict, key) => {
      dict[key] = !this.state[key];
      return dict;
    }, {}));
  }
}

@Component({
  selector: 'my-app',
  template: ``,
})
export class App {
  constructor() {
    const toggles = new ToggleTests();
    toggles.toggle1('one', 'two');
    toggles.toggle2('one', 'two');
    toggles.toggle3('one', 'two');
    toggles.toggle4('one', 'two');
  }
}

@NgModule({
  imports: [ BrowserModule ],
  declarations: [ App ],
  bootstrap: [ App ]
})
export class AppModule {}

c:\test\12200>tsc --v
Version 2.2.0-dev.20161113

c:\test\12200>tsc
app.ts(35,5): error TS2346: Supplied parameters do not match any signature of call target.

@sasxa
Copy link
Author

sasxa commented Nov 13, 2016

It works for me too when I isolate it. Not sure what might be causing the error in my project...

c:\dev\projects\.examples\12200>dir
 Volume in drive C has no label.
 Volume Serial Number is 1682-77A6

 Directory of c:\dev\projects\.examples\12200

13.11.2016.  19.47    <DIR>          .
13.11.2016.  19.47    <DIR>          ..
13.11.2016.  19.50             1.818 main.js
13.11.2016.  19.46               694 main.ts
13.11.2016.  19.45    <DIR>          node_modules
13.11.2016.  19.50               148 package.json
               3 File(s)          2.660 bytes
               3 Dir(s)  37.805.133.824 bytes free

c:\dev\projects\.examples\12200>type package.json
{
  "main": "main.js",
  "scripts": {
    "test": "tsc main.ts && node ."
  },
  "devDependencies": {
    "typescript": "^2.2.0-dev.20161113"
  }
}

c:\dev\projects\.examples\12200>type main.ts
class ToggleTests {
  state: any = {};

  toggle1(key: string) {
    console.log({ ...this.state, [key]: !this.state[key] });
  }

  toggle2(...keys: string[]) {
    console.log({ ...this.state, [keys[0]]: !this.state[keys[0]] });
  }

  toggle3(...keys: string[]) {
    console.log(keys.reduce((dict, key) => ({ ...dict, [key]: !this.state[key] }), {}));
  }

  toggle4(...keys: string[]) {
    console.log(keys.reduce((dict, key) => {
      dict[key] = !this.state[key];
      return dict;
    }, {}));
  }
}

const toggles = new ToggleTests();

toggles.toggle1('one');
toggles.toggle2('one', 'two');
toggles.toggle3('one', 'two');
toggles.toggle4('one', 'two');

c:\dev\projects\.examples\12200>npm run test

> @ test c:\dev\projects\.examples\12200
> tsc main.ts && node .

{ one: true }
{ one: true }
{ one: true, two: true }
{ one: true, two: true }

@DanielRosenwasser DanielRosenwasser added the Needs More Info The issue still hasn't been fully clarified label Nov 13, 2016
@sasxa
Copy link
Author

sasxa commented Nov 13, 2016

In the error I got, second line:

    at getObjectLiteralIndexInfo (~\node_modules\typescript\lib\typescript.js:34000:48)

points to this function in source code:

        function getObjectLiteralIndexInfo(node, properties, kind) {
            var propTypes = [];
            for (var i = 0; i < properties.length; i++) {
                if (kind === 0 /* String */ || isNumericName(node.properties[i].name)) { // <-- this is line 34000,
                    propTypes.push(getTypeOfSymbol(properties[i]));
                }
            }
            var unionType = propTypes.length ? getUnionType(propTypes, /*subtypeReduction*/ true) : undefinedType;
            return createIndexInfo(unionType, /*isReadonly*/ false);
        }

If I change this line to:

                if (kind === 0 /* String */ || isNumericName(properties[i].name)) { // <-- this is line 34000,

code compiles without errors and works in my app.

Don't know how this works internally, or where is this in source code, but hopefully it will help (:

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2016

We need to know why this is happening. why is the node undefined at runtime. if you can debug locally, when the exception happen try ts.getSourceFileOfNode(node) and that should point you to which file caused the issue. then we can narrow the error better.

@sasxa
Copy link
Author

sasxa commented Nov 14, 2016

It seams "isolatedModules": true is the cause of the error. I created a this repo: https://github.com/sasxa/TS12200.git to illustrate the issue. If you set "isolatedModules": true and run gulp it will cause this error.

@mhegazy mhegazy added this to the TypeScript 2.1.3 milestone Nov 14, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2016

@sandersn can you take a look.

@sandersn
Copy link
Member

@sasxa found the source of the bug. getObjectLiteralIndexInfo assumes that node.properties is the same length as its properties parameter. Since spreads break up the object literal into multiple literals, this assumption no longer holds. Instead, getObjectLiteralIndexInfo needs an array of property syntax nodes that matches the array of symbols, or else an offset to index into the original list of nodes. Or it might be able to use valueDeclaration to get back to the property node from the symbols in properties.

@mhegazy mhegazy added Bug A bug in TypeScript and removed Needs More Info The issue still hasn't been fully clarified labels Nov 14, 2016
@sandersn
Copy link
Member

Fix is up at #12248

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Nov 15, 2016
@sasxa
Copy link
Author

sasxa commented Nov 16, 2016

Just to confirm it works in 2.2.0-dev.20161116...

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants