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

Should not always add 'use strict' on node modules #1031

Closed
qraynaud opened this issue May 18, 2014 · 25 comments
Closed

Should not always add 'use strict' on node modules #1031

qraynaud opened this issue May 18, 2014 · 25 comments

Comments

@qraynaud
Copy link

Right now, traceur automatically add a "use strict" statement on generated node modules (first line as should be). It can break existing modules that do not use strict mode but make use of parts of ES6 (like some koa modules).

I believe (but I might be wrong) that it would be better to keep the "use strict" statement only if the user had it there in the first place.

@matthewrobb
Copy link

If you use the makeDefault method for require overloading then you can specify a pattern or filter for what should be traceur loaded and what shouldn't. If you do this then only traceur module's end up with the added "use strict" which is absolutely correct from an ES6 stand point.

If koa is using es6 modules then it is using "strict mode" implicitly according to the spec.

@domenic
Copy link
Contributor

domenic commented May 18, 2014

To be explicit, ES6 modules are always in strict mode. So if you are writing scripts (which do not allow import and export and module statements), you can stay in sloppy mode, but if you are writing modules, then you are opting in to strict mode automatically.

@qraynaud
Copy link
Author

OK. Thanks a lot for the explanation. I had a hunch it was something like that. It would be cool if the koa modules where developed with 'use strict' on top of file because node does not seem to enforce it when --harmony is activated.

It means that, basically, it is allowing users to run code with some ES6 features but not the enforcements made by use strict. This is sad.

@domenic
Copy link
Contributor

domenic commented May 18, 2014

You can still use ES6 features while in sloppy mode, as long as you are authoring scripts, not modules. By default Node code is all in scripts; I don't know of a way to run Node code in modules, probably because V8 doesn't support modules yet.

@matthewrobb
Copy link

It's completely valid to use some ES6 features without "strict mode" and it's important to know that node uses it's own brand of CommonJS style module system and Traceur providing an easy means of hooking into that is really a special Traceur feature and nothing more.

In a world where ES6 features are all available natively this "issue" still exists and by design. Keep in mind that you do not need your entire program to be in strict mode to gain the benefits of strict mode where it's been declared.

var global = this;

function sloppy() {
    console.log(this === global);
}

function strict(both){
    "use strict";
    console.log(this === global);
    if(both){
        sloppy();
    }
}

sloppy(); // true
strict(); // false
strict(true); // false && true

@qraynaud
Copy link
Author

Then traceur should detect the presence of 'use strict' and add it back only if it was there first right?

@qraynaud qraynaud reopened this May 18, 2014
@domenic
Copy link
Contributor

domenic commented May 18, 2014

No. If you're compiling code to ES6 modules with Traceur, then that code needs to have strict semantics. Since traceur compiles ES6 modules -> ES5 scripts, it needs to put the scripts in strict mode via the only way that is possible, namely "use strict";.

If you are compiling ES6 scripts to ES5 scripts, then Traceur will not put "use strict"; there, as noted in @matthewrobb's original comment.

@matthewrobb
Copy link

Only if you have module compilation off. Otherwise Traceur is transcompiling your source AS IF it was an ES6 module which like has already been stated, automatically strict mode.

NOTE: Class bodies are also strict mode

@qraynaud
Copy link
Author

I don't think I am compiling to ES6 modules. How do I check that?

@matthewrobb
Copy link

var traceur = require("traceur");

traceur.options.modules = true; // This is the default

@qraynaud
Copy link
Author

OK. I added traceur.options.modules = false and it is still adding "use strict".

@matthewrobb
Copy link

I'm sorry it looks like you need to add

traceur.options.modules_ = false;

as well.

@qraynaud
Copy link
Author

Same thing with traceur.options.modules_ = false; (I also kept modules = false)

@arv
Copy link
Collaborator

arv commented May 20, 2014

If we are compiling a Module production we are adding "use strict" since this is the most correct thing to do.

@qraynaud How are you compiling your code? If you are using the command line compiler try passing the files as --script.

@qraynaud
Copy link
Author

No I'm not using the command line compiler. i'm using the compiler on require via node.

@qraynaud
Copy link
Author

@arv: any other ideas? Right now I would really like to include modules created by others in my project that don't support and don't work in strict mode. It would be cool if traceur could work everywhere.

That said, I would gladly write the patch myself but I would really need some help because the codebase is somewhat complex already.

@johnjbarton
Copy link
Contributor

Currently the compile() function in src/node/api.js calls var tree = parser.parseModule(); and transforms to create modules. If you want to parse and transform 'scripts' (ES6 speak for not-es6-modules), then you will need to refactor compile() to isolate the module parts then create a new API function, say script() that calls the common parts but parse and transforms for 'scripts'. That will result in code without the default "use strict".

@arv
Copy link
Collaborator

arv commented May 29, 2014

How about changing compile to use parseScript if the modules options is false?

@qraynaud
Copy link
Author

Sorry for the very long lapse since my last answer. I'm getting back to this and I found out that there is already something like that in the codebase of traceur (at least in recent versions). In src/Compiler.js at line 132, I can read:

var tree = mergedOptions.modules ? parser.parseModule() : parser.parseScript();

If I'm adding just before this line mergedOptions.modules = false, everything is working properly.

My problem is I can't find a way to get modules to false. It is always equal to commonjs whatever I do. So I tried something else and looked all values. I also see that blockBinding = false even if I added traceur.options.blockBinding = true before calling traceur.require.makeDefault().

I don't know how to pass options anymore to traceur. Has this changed recently? (I'm on 0.0.49). The documentation doesn't talk much about this...

@qraynaud
Copy link
Author

Okay, after backporting some code from the future 0.0.50 about options in makeDefault to my code, it works!

@qraynaud
Copy link
Author

For others that might get here, here is what I wrote to get everything working properly:

var traceur = require('traceur');
traceur.require.makeDefault(filterFn, {
  blockBinding: true,
  modules: false,
});

@qraynaud
Copy link
Author

(hope 0.0.50 is released on NPM soon)

@nottoseethesun
Copy link

Are there any other side effects to turning off modules and turning on blockBinding, other than preventing Traceur from inserting 'use strict'; statements?

In my project, I want developers to insert 'use strict' at the top of their files, so that it's clear that the source code is in use-strict mode to anyone who looks at the file (and also to any js IDE tooling). Currently, the project lint throws warnings due to Traceur inserting excess 'use strict's.

So I want my source files to have 'use strict'; in them, but don't want Traceur to change its behavior from the default. Is this possible? If not, why?

@johnjbarton
Copy link
Contributor

Please don't ask questions on closed issues. Ask on the newsgroup or open a new issue. Refer to this one if it helps.

@nottoseethesun
Copy link

Done: #1427

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants