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

TS >=4.1: Cannot assign because it is not a variable (require does not work with let anymore) #41628

Closed
sk- opened this issue Nov 21, 2020 · 3 comments
Assignees
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Design Limitation Constraints of the existing architecture prevent this from being fixed Domain: JavaScript The issue relates to JavaScript specifically

Comments

@sk-
Copy link

sk- commented Nov 21, 2020

Starting with TS 4.1 and also in TS 4.2, it seems that required code is frozen and cannot be reassigned.

We had some code that would require a JSON and conditionally overwrite it, however that code is no longer valid. See example below for a minimal reproduction.

I couldn't find anything in the release notes or breaking changes.

TypeScript Version: 4.1.2

Search Terms:
require json cannot assign let

Code

let a = require('./package.json');
a = {...a};

Expected behavior:
No warnings should be raised for this code. Alternatively at the least I would expect another warning about defining a with let.

Actual behavior:

 Cannot to 'a' because it is not a variable.

Playground Link:
https://www.typescriptlang.org/play?ts=4.2.0-dev.20201121&useJavaScript=true#code/DYUwLgBAhhC8ECcQEcCuBLJAKA5AOgHoAHKAYwGsoBzEPAKwGcB7AOxwEoBuAWACgZ4AbzwioAXz5A

Related Issues:

@sk- sk- changed the title TS >4.1: Cannot assign because it is not a variable (require does not work with let anymore) TS >=4.1: Cannot assign because it is not a variable (require does not work with let anymore) Nov 21, 2020
@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically checkJs Relates to checking JavaScript using TypeScript labels Nov 24, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.2.0 milestone Nov 24, 2020
@DanielRosenwasser
Copy link
Member

This might need to be ported back to a 4.1.3 release.

@a-tarasyuk
Copy link
Contributor

I think that's because TS binds let x = require('y') statements the same way as import x = require('y'); - as an alias - #39770. Not sure, maybe the binder should only set the alias flag for const variables

if (isInJSFile(node) && isRequireVariableDeclaration(node, /*requireStringLiteralLikeArgument*/ true) && !getJSDocTypeTag(node)) {
declareSymbolAndAddToSymbolTable(node as Declaration, SymbolFlags.Alias, SymbolFlags.AliasExcludes);

@sandersn
Copy link
Member

sandersn commented Feb 2, 2021

I looked at the frequency of const, let and var in conjunction with require and found that const outnumbers let about 100 to 1. var outnumbers let more than 200 to 1. Unfortunately, I couldn't think of an easy way to find re-assignments without compiling all the code I searched through. So I looked at the user tests, which are compiled. But it didn't have any errors like this.

I can think of two possible fixes:

  1. Correctly allow assignments to both let and var, since both are technically mutable (although it's rare in practice).
  2. Allow assignment to let only, since it's rare and it lets us keep the improved declaration emit and goto-def for var, which is more common.

And of course there's always:
0. Do nothing.

(1) causes us to lose aliasing for var, which is a big loss. (2) makes another corner of JS handling more complex. Since this only affects projects with checkJs: true and use var/let with require to mutate the binding, I think the best course is to do nothing. I would recommend // @ts-ignore for cases where you need to write this in a checked JS file.

Side note: I don't think we've noted breaking changes to checkJs features in the past. We need to improve that. And of course I just missed that this would be a break at the time.

@sandersn sandersn closed this as completed Feb 2, 2021
@sandersn sandersn added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Design Limitation Constraints of the existing architecture prevent this from being fixed Domain: JavaScript The issue relates to JavaScript specifically
Projects
None yet
Development

No branches or pull requests

4 participants