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

Emit 'use strict' code for input defined via ES6 modules #3576

Closed
IgorMinar opened this issue Jun 19, 2015 · 27 comments
Closed

Emit 'use strict' code for input defined via ES6 modules #3576

IgorMinar opened this issue Jun 19, 2015 · 27 comments
Labels
Breaking Change Would introduce errors in existing code Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@IgorMinar
Copy link

ES6 modules default to running the code in the module in the strict mode. When transpiling this input into ES5 the strict mode should be preserved.

As of Typescript 1.5 beta this is not the case:

input:

export class Foo{}

transpile with: tsc --module commonjs test.ts

actual output:

var Foo = (function () {
    function Foo() {
    }
    return Foo;
})();
exports.Foo = Foo;

expected output:

'use strict';

var Foo = (function () {
    function Foo() {
    }
    return Foo;
})();
exports.Foo = Foo;

The same issue affects output for amd and system module formats.

@danquirk
Copy link
Member

I think #1135 covers this topic

@mhegazy
Copy link
Contributor

mhegazy commented Jun 19, 2015

There are two parts, 1. parse module in strict mode, and this is covered by #1135, and 2. emit a "use strict"; directive at the top of the file. i do not think 2 is needed if 1 is in place. @IgorMinar is this correct?

@IgorMinar
Copy link
Author

I think you need both.

  1. allows you to catch issues during static analysis
  2. ensures that the code runs with the semantics of the strict mode

good example of where 2. is needed is assigning to non-writable or frozen properties of an object, which can't be reliably detected statically (AFAIK). At runtime, such assignment fails with an error in strict mode, but in sloppy mode the error is silenced. I think it is important that this behavior is preserved between ES6 and ES5 output.

@DanielRosenwasser
Copy link
Member

I wouldn't necessarily say it's "needed", but if you get strict mode for free in an ES6 module, is there any actual harm of writing "use strict"; at the top of your file?

As a side note, this would be helpful in verifying our JS output by ensuring that a module is properly emitted in strict mode.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 20, 2015

Also, not everyone writing modules in TypeScript are expecting ES6 behaviours and auto-injecting "use strict;" into ES5 will likely cause breakage and surprise.

@DanielRosenwasser
Copy link
Member

injecting "use strict;" into ES5 will likely cause breakage and surprise.

This is my fear - that someone will concat their output files in such a way that this causes everything to break. Though I don't know enough about scenarios in which one would do this with external modules.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Aug 10, 2015
@xLama
Copy link

xLama commented Aug 28, 2015

I guess It could be a transpiler parameter

@speigg
Copy link

speigg commented Sep 30, 2015

The spec mandates that es6 modules be executed in strict mode: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-strict-mode-code

strict-mode javascript is not a subset of non-strict-mode, it has intentionally different semantics. Transpiling from es6 modules to es5 javascript strictly requires injecting "use strict;", it's really not an option.

@speigg
Copy link

speigg commented Sep 30, 2015

@DanielRosenwasser es6 modules cannot be simply concatenated together. Any bundler has to ensure that each module has it's own scope.

@DanielRosenwasser
Copy link
Member

@speigg I realized this as I opened this issue and re-read my comment.

@weswigham is doing some work in this area. I believe the new behavior is going to be that we only expect strict mode if the module target is ES6 (a new module target that is separate from the language target).

This is because if someone is purely targeting something like CommonJS, then the strict mode requirement no longer really applies, however, @weswigham can probably explain more.

@speigg
Copy link

speigg commented Sep 30, 2015

Not sure what you mean by "expecting strict mode". If the input is es6 modules and the output is also es6 modules, then "use strict;" doesn't need to be injected, it's implied. If the input is es6 modules and the output is anything other than es6 modules, then "use strict;" must be injected (even if the output is CommonJS). If the input is not es6 modules, then "use strict;" is not required (or implied).

@kitsonk
Copy link
Contributor

kitsonk commented Sep 30, 2015

If the input is es6 modules and the output is anything other than es6 modules, then "use strict;" must be injected (even if the output is CommonJS). If the input is not es6 modules, then "use strict;" is not required (or implied).

The challenge though is in TypeScript you can mix the inputs but target a specific output:

import bar = require('bar');
import foo from 'foo';

What about that use case? Also, just because a developer is using ES6 module syntax, but targeting AMD/UMD/CommonJS that they are expecting strict behaviour.

@weswigham
Copy link
Member

@speigg I opened #4963 to start a discussion on this. Our present behavior is to require strict mode everywhere when targeting any module form, however, we do not emit a "use strict"; prologue in our module emit for non-es6 module targets to match this. We could do one of two things to rectify this mismatch:

  • Parse non-es6 module forms in non-strict mode (So if --module is not es6, we don't parse modules in strict mode)
  • Emit "use strict"; prologues in non-es6 modules

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Nov 24, 2015
@mhegazy mhegazy added this to the TypeScript 1.8 milestone Nov 24, 2015
@mhegazy mhegazy removed the In Discussion Not yet reached consensus label Nov 24, 2015
@IgorMinar
Copy link
Author

Awesome! Thanks Wesley.
On Nov 24, 2015 1:59 PM, "Wesley Wigham" notifications@github.com wrote:

Closed #3576 #3576 via
#5765 #5765.


Reply to this email directly or view it on GitHub
#3576 (comment).

@kitsonk
Copy link
Contributor

kitsonk commented Jan 5, 2016

I suspected there would be breakage with this, I just didn't expect it to happen to me. In Dojo we have a module (core/global) that allows us to grab a reference to the global scope. Because all modules now are being emitted with the "use strict"; prologue, this module is returning undefined now.

Anyone have a suggestion for how to grab a reference to the global scope in TypeScript modules in 1.8+?

@DanielRosenwasser
Copy link
Member

@kitsonk you could give this a try http://stackoverflow.com/a/3277192/4386952

TL;DR:

const globalObject: any = Function("return this")();

@dasa
Copy link

dasa commented Jan 28, 2016

Is there a way to turn off the "use strict" prologue emit? This is a problem for us when working with Dojo 1.x and making calls to this.inherited(arguments).

@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

This is a breaking change, mainly to align with the ES6 spec. that code would not run in ES6 in the future either.

@jkieboom
Copy link

This seems unexpected to me. If I'm emitting ES5 code then this should not be implicitly enabled, as I'm targeting ES5 and not ES6. I understand that my code is then not ES6 compliant, but that's the whole point of transpiling it anyway. Doing this unconditionally for the sake of ES6 strictness seems pedantic, especially because this will break actual/running code. Also, if you are worried about writing TS modules that are not ES6 compliant, then would it not be better to have the compiler error out when the target is ES6 and you use things that do not work?

@weswigham
Copy link
Member

You'll find that TS now aligns with other module transpilers (like babel) here - when transpiling modules, we parse the code in strict mode, per spec, and emit a use strict prologue to ensure runtime semantics match compile time ones.

When you write TS, you are not writing ES5 code - you are writing TS code, which happens to be a superset (or, strives to be) of ES6/7, which then gets transpiled to equivalent ES3/5/6. When you use ES6 module syntax, you also get ES6 module rules - it was an oversight which could have caused unexpected runtime behavior, for example:

Second, strict mode code doesn't alias properties of arguments objects created within it. In normal code within a function whose first argument is arg, setting arg also sets arguments[0], and vice versa (unless no arguments were provided or arguments[0] is deleted). arguments objects for strict mode functions store the original arguments when the function was invoked. arguments[i] does not track the value of the corresponding named argument, nor does a named argument track the value in the corresponding arguments[i]. (from MDN)

What this means is that in TS prior to 1.8, you could write this:

export function multiplyThenAdd(num){
  num = num * num;
  return num + arguments[0];
}

with the expectation that calling multiplyThenAdd(2) should return 6, since you're writing a module and TS enforces strict mode in modules - but alas, you'd have been misled, as you'd witness a result of 8, since TS didn't set strict mode to on at runtime! This fixes that.

@dylans
Copy link

dylans commented Jan 29, 2016

I would think it would be better to discourage people from writing code that uses the arguments object after you've started manipulating the arguments or changing the value of the arguments that are passed in. Obviously asking the world to change how they write code isn't going to happen.

As much as I hate to suggest yet another compiler setting, I would think that an opt-in setting to turn off the emitting of use strict for either all transpiled code, or per module, would allow people that understand the consequences to proceed.

As noted, the use case is compatibility when authoring code with TS that extends legacy JS code where use strict is not permitted. For example, in Dojo 1, the declare syntax usesarguments.callee to walk the inheritance chain.

@dasa
Copy link

dasa commented Feb 2, 2016

I've created a new issue for adding a compiler setting at #6819

@jeffreymorlan
Copy link
Contributor

dojo's this.inherited calls are pretty easy to fix, you can just give it an explicit {callee} object:

// strict mode incompatible
foo: function() {
    this.inherited(arguments);
}

// strict mode compatible
foo: function foo() {
    this.inherited({callee: foo}, arguments);
}

@dylans
Copy link

dylans commented Feb 2, 2016

dojo's this.inherited calls are pretty easy to fix, you can just give it an explicit {callee} object

Unfortunately most of Dojo's "declared classes" (objects that use dojo/_base/declare to setup a mixin-based approach to inheritance) are not named functions, and changing thousands of such classes across Dojo isn't going to happen quickly enough, and certainly not for anyone locked on a particular version of Dojo.

@jeffreymorlan
Copy link
Contributor

You don't have to change dojo at all, only your own declare() classes.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 3, 2016

@jeffreymorlan while it is true, only those writing TypeScript modules which are going to be emitted in strict mode would have to worry about this, it is still an arbitrary requirement that would make anyone migrating a Dojo 1 project to TypeScript probably think it wasn't worth the effort, since almost no one would be declaring their custom class methods as named functions. Many people who use Dojo 1 have large, complex, custom classes, and having to touch all that code to just start the process of migrating to TypeScript would be untenable.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 8, 2016

For reference, the emit is now configurable via --noImplicitUseStrict compiler flag via #6819.

@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
Breaking Change Would introduce errors in existing code Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests