Skip to content

Type inference broken in JS files - Behavior differs from typescript #30009

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
jbwyme opened this issue Feb 20, 2019 · 9 comments
Closed

Type inference broken in JS files - Behavior differs from typescript #30009

jbwyme opened this issue Feb 20, 2019 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jbwyme
Copy link

jbwyme commented Feb 20, 2019

TypeScript Version: Version 3.3.3, Version 3.4.0-dev.20190220

Search Terms:
Object mutation, Inferred type, JSDoc

Code

// @ts-check

/** @returns {{prop: string}} */
function buildParams() {
  const params = {
    prop: `val`,
  };
  params.filter = {mutation: 1};
  return params;
}

buildParams();

Expected behavior:
Should error on object mutation and/or return type not matching

Actual behavior:
No errors

Example of bug

local:test$ cat test.js
// @ts-check

/** @returns {{prop: string}} */
function buildParams() {
  const params = {
    prop: `val`,
  };
  params.filter = {mutation: 1};
  return params;
}

buildParams();

local:test$ node_modules/typescript/bin/tsc --noEmit --allowJs test.js 

Proof that type checking works in general for js files

local:test$ cat test.js 
// @ts-check

/** @returns {{prop: string}} */
function buildParams() {
  const params = {
  };
  params.filter = {mutation: 1};
  return params;
}

buildParams();

local:test$ node_modules/typescript/bin/tsc --noEmit --allowJs test.js 
test.js:8:3 - error TS2741: Property 'prop' is missing in type '{ filter: { mutation: number; }; }' but required in type '{ prop: string; }'.

8   return params;
    ~~~~~~~~~~~~~~

  test.js:3:16
    3 /** @returns {{prop: string}} */
                     ~~~~
    'prop' is declared here.


Found 1 error.

Same logic in typescript

local:test$ cat test.ts
type Data = {
  prop: string,
}

function buildParams(): Data {
  const params = {
    prop: `test`,
  };
  params.filter = {mutation: 1};
  return params;
}

buildParams();

local:test$ node_modules/typescript/bin/tsc --noEmit test.ts
test.ts:9:10 - error TS2339: Property 'filter' does not exist on type '{ prop: string; }'.

9   params.filter = {mutation: 1};
           ~~~~~~


Found 1 error.
@jbwyme jbwyme changed the title Inferred types broken in JSDoc Type inference broken in JSDoc - Behavior differs from typescript Feb 20, 2019
@jbwyme jbwyme changed the title Type inference broken in JSDoc - Behavior differs from typescript Type inference broken in JS files - Behavior differs from typescript Feb 21, 2019
@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 27, 2019
@RyanCavanaugh
Copy link
Member

There are several intentional differences between JS and TS files as far as typechecking goes; this is one of them - you can "tack on" new properties to an object while in the same scope it was declared, i.e. this is still an error:

// @ts-check

/** @returns {{prop: string}} */
function buildParams() {
    const params = {
        prop: `val`,
    };
    return params;
}

const p = buildParams();
// Error
p.filter = { mutation: 1 };

@nojvek
Copy link
Contributor

nojvek commented Feb 27, 2019

That's really confusing to be honest. Could you explain why this is working as intended. It clearly is not catching a bug one would expect typescript should. I would expect TS/JS typing to be functionally equivalent. Isn't that the whole value proposition?

Kind ask: At-least give us some sort of option/workaround to enable stricter js checking which is functionally equivalent to TS.

@RyanCavanaugh
Copy link
Member

Could you explain why this is working as intended

There are several intentional differences between JS and TS files as far as typechecking goes; this is one of them - you can "tack on" new properties to an object. This was based on feedback of trying out the ts-check mode on many JS codebases.

At-least give us some sort of option/workaround to enable stricter js checking which is functionally equivalent to TS.

Certainly open to a new suggestion on that.

@jbwyme
Copy link
Author

jbwyme commented Mar 1, 2019 via email

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@tdumitrescu
Copy link

Really typescript-bot?

@jbwyme
Copy link
Author

jbwyme commented Mar 3, 2019

@RyanCavanaugh Should I create a separate issue to open up a discussion about the goals of typescript vs @ts-check? My concern is that if @ts-check is some weird subset of / divergent from typescript, it will not be usable as a replacement and will in fact result in a lot of unexpected behavior thus eliminating the utility of it.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Mar 4, 2019

I have to document this anyway so I'll start here - here are the differences in typechecking between JS and TS files:

  • JS Doc will change the types of parameters/etc in JS files, but not in TS files
  • The default type argument in a generic call with no inferences is any instead of {}
  • Variables in JS always get control-flow initialization treatment instead of bare initializer treatment (in TS, this requires noImplicitAny to be on)
  • Variables initialized with object literals are subject to expando treatment
  • JS functions can be invoked with new in more cases
  • Object literals in JS get a contextual this value (in TS, noImplicitThis must be enabled for this to happen)
  • A type reference in JS files (in JS Doc) may omit type parameters, which are defaulted to any
  • JS files detect prototype assignment based classes
  • JS files show properties from all types in a union in completion lists
  • A call to require is assumed to be a module import
  • Certain other CommonJS assignments are automatically detected

@RyanCavanaugh
Copy link
Member

So yeah we can take a suggestion on something that e.g. turns off all of that behavior, or maybe just the certain aspects that you might find objectionable. That said, all of these were added on purpose based on user feedback and investigation of the ergonomics of writing passing code without the ability to write type assertions, type arguments, or type annotations, so it's worth thinking hard about what the exact trade-off being made is.

"different inputs produce different behavior" is not a prima facie justification - some longer description of what you'd want from this mode would be useful, since simply renaming JS to TS is also a way to get the TS behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants