Skip to content

v3.7.2 not compiling valid Javascript #35149

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
ghost opened this issue Nov 17, 2019 · 14 comments
Closed

v3.7.2 not compiling valid Javascript #35149

ghost opened this issue Nov 17, 2019 · 14 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ghost
Copy link

ghost commented Nov 17, 2019

// test.js
function something({one} = {}) {}
$ tsc -v; tsc test.js --allowJS --checkJS --noEmit
Version 3.7.2
test.js:2:21 - error TS2525: Initializer provides no value for this binding element and the binding element has no default value.

2 function something({one} = {}) {}
                      ~~~


Found 1 error.
@kroleg
Copy link

kroleg commented Nov 17, 2019

Your javascript is not exactly "valid". If you remove --checkJS param it will comile without errors

@ghost
Copy link
Author

ghost commented Nov 17, 2019

What's not valid about it? It runs in node and chrome perfectly. I want to keep checkJS because I have some files with jsdocs I'd like to validate. I don't want to have to manually put each file I want to validate in include, I just want to validate all files in a folder and I thought typescript works with javascript, seems not to be the case.

@fatcerberus
Copy link

checkJs means you’re type-checking it exactly like a .ts file, just without type annotations. Any type inference semantics and errors that apply to TS code also apply to checkJs’d JavaScript. This is, in fact, a type error because you’re trying to destructure a one property out of a type ({}) which is known statically not to have it.

@ghost
Copy link
Author

ghost commented Nov 17, 2019

So Typescript does not compile valid Javascript then? Why isn't it an implicit any? I thought the promise of Typescript is that it works out of the box with JS.

@AnyhowStep
Copy link
Contributor

It worked out of the box and told you there was something suspicious it sniffed from your code.

@ghost
Copy link
Author

ghost commented Nov 17, 2019

there's nothing suspicious, something({one} = {}) is a valid and common javascript pattern. why would I want an error on it? I thought the whole point of TS is I don't have to rewrite my entire JS codebase.

@milesj
Copy link

milesj commented Nov 17, 2019

@fatcerberus @kroleg answered your question above. something({one} = {}) is valid JS but not valid TS, and when you pass --checkJS, it validates your code as if it was TS.

Are you purposefully ignoring everyones answer here?

@kroleg
Copy link

kroleg commented Nov 17, 2019

@dave-dm there is something suspicious. By default TS treats all object properties as required, so in case of something({one} = {}) one is missing in {}.
One solution will be to add jsdoc to tell TS that one is optional:

/**
 * @param {object} param0
 * @param {string} [param0.one]
 */
function something({one} = {}) {}

Another is to add ts-ignore directive

// @ts-ignore
function something({one} = {}) {}

@apalaniuk
Copy link

I doubt it strictly has to do with type checking via inferred types (if it was, I think you'd see Property 'one' does not exist on type '{}'. ts(2339)) instead):

// error TS2525: Initializer provides no value for this binding element
// and the binding element has no default value.
function something({ one } = {}) {}

// no error!
function something2({ one }) {}

// no error!
function something3(someObj = {}) { // inferred: `(parameter) someObj: {}`
    let { one } = someObj;
}

// no error!
const someObj = {}; // inferred: `const someObj: {}`
let { someProp } = someObj;

All equally suspicious/statically verifiable, with the same inferred types. In fact, checking the same file contents in a .ts file and the most permissive tsconfig results in ts(2339) for all the above cases (save for the no-default-value case), so it seems the claim that checkJs enforces the same checks as in .ts files using the same inferences isn't correct. It's not as though there haven't been similar apparent issues in the same ballpark before, which is a vaguely similar case, so I don't think it's unreasonable to be a bit surprised at this behaviour, or for it to be dismissed so readily.

@fatcerberus
Copy link

Maybe so, but the point remains: TS only promises it will compile JS out of the box (which it does—it even emits code in the presence of type errors!), it doesn’t promise it will typecheck. checkJs isn’t just a “Verify the JSDoc” mode, it also attempts to infer types and verifies those too. It’s not even clear in many cases where the explicit documentation ends and the inferences begin.

We’ll just have to wait to see what a TS team member has to say about this, but I still suspect this is by design.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 18, 2019

I think (maybe talking out of my ass?) there's also some stuff that TS only does syntactically, and not semantically for checkJs.

So, some constructs give errors because TS can identity the problem syntactically (someone specifically coded a check for it, that looks at the syntax only).

While other constructs (that express the same thing) don't give errors because the check can't just be done syntactically (or no one coded the syntax-only check).

checkJs is an overall more relaxed mode that doesn't always perform all checks.

@ghost
Copy link
Author

ghost commented Nov 18, 2019

Well my use case is only to check jsdocs in all my .js files, and ignore all valid javascript. If that's not possible, that is unfortunate.

@kroleg adding comments doesn't work for my use case. I have a huge codebase and only want jsdocs in a few places. It has to let valid javascript pass through without any addition.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Nov 26, 2019
@RyanCavanaugh
Copy link
Member

If you just want to validate that your JS Doc matches your parameter names, checkJs would be the wrong thing - it is for doing much more than that.

@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.

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

7 participants