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

External variable is considered a constant or read-only wrongly #6751

Closed
plantain-00 opened this issue Jan 30, 2016 · 12 comments
Closed

External variable is considered a constant or read-only wrongly #6751

plantain-00 opened this issue Jan 30, 2016 · 12 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@plantain-00
Copy link
Contributor

error message when run tsc

error TS2450: Left-hand side of assignment expression cannot be a constant or a read-only property.

a.ts
export let value = 1;
b.ts
import * as a from "./a";

function load() {
    a.value = 2;
}
tsconfig.json
{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs"
    }
}
environment
>tsc -v
Version 1.9.0-dev.20160129
@Arnavion
Copy link
Contributor

That is how ES6 modules work - imports are constant. Only the module that exports them can change them, which then propagates to all modules that import them.

a.ts

export let value = 1;

export function setValue(newValue: number) {
    value = newValue;
}

b.ts

import * as a from "./a";

function load() {
    a.setValue(2);
}

@plantain-00
Copy link
Contributor Author

@Arnavion I'm afraid not that.
Constant works like this:

const b = {
    c: 1
};
b = null; // I can not do this
b.c = 2; // I can do this

The generated js I expects is:
a.js:

"use strict";
exports.value = 1;

b.js:

"use strict";
var a = require("./a");
function load() {
    a.value = 2;
}

If I invoke load() in node.js(I did this earlier), it works, the value of a's value changes.

BTW, the code works with typescript@1.7 and typescript@1.8.0-dev.20160117, but not work with typescript@1.9.0-dev.20160129, I upgraded typescript today, then I find the error.

@Arnavion
Copy link
Contributor

Yes, if you compile ES6 to ES5 require then you get the behavior you expect.

That does not mean ES6 modules allow it. As I said the import binding is immutable and is not allowed to be modified by ES6 spec.

The reason it worked before and errors now is that it was a long-standing bug #2456 that was only fixed a few days ago by #6532

@plantain-00
Copy link
Contributor Author

Alright, I will use require instead.

@ielcoro
Copy link

ielcoro commented Sep 9, 2016

I still don't get why it enforces es6 module format when using "module": "amd" in tsconfig.json. I have a perfectly working durandal application that I can't migrate to TypeScript 2.0 because of this, as durandal declaration files, as many others, use modules with exported vars:

declare module 'durandal/binder' {
    interface BindingInstruction {
        applyBindings: boolean;
    }

    /**
     * Called before every binding operation. Does nothing by default.
     * @param {object} data The data that is about to be bound.
     * @param {DOMElement} view The view that is about to be bound.
     * @param {object} instruction The object that carries the binding instructions.
    */
    export var binding: (data: any, view: HTMLElement, instruction: BindingInstruction) => void;

    /**
     * Called after every binding operation. Does nothing by default.
     * @param {object} data The data that has just been bound.
     * @param {DOMElement} view The view that has just been bound.
     * @param {object} instruction The object that carries the binding instructions.
    */
    export var bindingComplete: (data: any, view: HTMLElement, instruction: BindingInstruction) => void;

When using those declaration files, all the imports, that previously worked fine, now import readonly constants, so this will not compile:

import * as binder from 'durandal/binder';

`binder.binding = (obj, view) => {
                        //do something
                    };`

Despite, as specified in tsconfig.json, all the imports being transpiled to amd format, so in the final JavaScript the imports end up being writable:

define(["require", "exports",  'durandal/binder'], function (require, exports, binder,) { ... }

Shouldn't be allowed to have writable imports when using amd or other module formats different from the ts specification?

Also, note that in this example changing all the imports (despite the hard work) to require does not work either, as you can have a typed variable from of a module called 'durandal/binder':

var binder : "durandal/binder" = require('durandal/binder')
//binder ends up being literal type 

var binder : durandal/binder = require('durandal/binder')
//this expression does not compile

var binder = require('durandal/binder');
//binder ends up being of type any`

@RyanCavanaugh
Copy link
Member

You can write

import binder = require('durandal/binder')

The suggestion that we ignore ES6 semantics when you use ES6 syntax seems to miss the point of downleveling. If you want AMD/CommonJS behavior, use the CommonJS-oriented syntax (import / require). Allowing you to do things using ES6 syntax which are specifically forbidden in ES6 is just setting you up for future failure when ES6 module implementations are widespread and you want to transition off of what is essentially a polyfill.

@ielcoro
Copy link

ielcoro commented Sep 12, 2016

Thanks @RyanCavanaugh

The import = require worked.

I can see the conflict with downleveling and changing semantics, however, I think the current state gives more headaches than what it solves. Using ES6 syntax in most of the places give us more power, like importing specific types from a module, that would need importing the whole module with AMD/CommonJS syntax, and the current downleveling works out the details (importing the whole module and referencing it accordingly). Also, migrating an application that uses import = require to ES6 modules, will be a pain, finding and changing all those imports for ES6 syntax.

Migrating to ES6 modules will be a pain anyway, as libraries must be changed to take into account the mutability of the exported variables from a module, so it could be made easier if there was a unified syntax to importing from typescipt, that adapts to the scenario in hand, reducing the weak Find and Replace when the migrating to ES6 modules (if that happens sometime in the near future ;-))

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Sep 12, 2016
@Bobris
Copy link

Bobris commented Sep 28, 2016

This is probably most significant breaking change when migrating from 1.8 to 2.0. Especially in tests when it is used for mocking.

@cervengoc
Copy link

Allowing you to do things using ES6 syntax which are specifically forbidden in ES6 is just setting you up for future failure when ES6 module implementations are widespread and you want to transition off of what is essentially a polyfill.

@RyanCavanaugh But if I understood correctly this error should only be suppressed if there is "target": "es5" in tsconfig.json. Whenever we might target native ES6 in the future, we will get the error and be forced to correct it. I think suppressing this error when targeting ES5 or lower is reasonable.

@RyanCavanaugh
Copy link
Member

I don't understand what's desirable about making the abstraction leaky on purpose. The entire point of downleveling is to make it appear as if ES6 modules were widely available, not to give you new syntax for ES5 behavior.

If you want ES5 semantics, there's an easy way to do that: Use ES5 syntax. If you use ES6 syntax, you're going to get ES6 semantics. That's not negotiable.

@cervengoc
Copy link

cervengoc commented Oct 4, 2016

It's a matter of viewpoint, I see this syntax as TypeScript syntax, not ES6 syntax. And as so, it should probably compile to ES5 without this error message. because I think the fact wether I want ES6 semantics or not is more related to the specified target. But if one declares it as ES6 syntax, then you are completely right. But then I'd probably disallow using this syntax when targeting ES5.

Anyway, this is just a respectful opinion, using require syntax is completely OK I think, just wanted to share my thoughts.

@netpedro-com
Copy link

netpedro-com commented Dec 26, 2016

Answering the initial question of @plantain-00, to supress error TS2450 message, just type (<any>a).value = 2; More info: http://stackoverflow.com/a/38833920/1882644

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

7 participants