Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Node 0.12 stable fails on strict mode #9187

Closed
lewispham opened this issue Feb 11, 2015 · 17 comments
Closed

Node 0.12 stable fails on strict mode #9187

lewispham opened this issue Feb 11, 2015 · 17 comments
Assignees
Milestone

Comments

@lewispham
Copy link

I found this error when I run node with node --use_strict myapp.js. What can I do in this case?

syntaxerroronstrictmode

@misterdjules
Copy link

If you need to load the crypto module and to pass the --use_strict flag to V8, I don't think there is anything you can do with node v0.12.0 to prevent this error.

May I ask why you need to pass --use_strict?

Adding this issue to the 0.12.1 milestone.

@misterdjules misterdjules added this to the 0.12.1 milestone Feb 11, 2015
@lewispham
Copy link
Author

@misterdjules Just because it's more convenient than inserting use strict string on top of each file. BTW, this flag --use_strict is pretty useless since several internal and external modules fail on strict mode. Node can fix its modules, but the external one. I really think Node should provide some standards which may be similar to ECMAScript. Otherwise, sooner or later, those external guys will mess it up.

@OrangeDog
Copy link

See also #7479.

@clintdw
Copy link

clintdw commented Feb 14, 2015

Maybe I don't fully comprehend what the --use_strict flag to V8 (via node) is intended for (internal testing for the V8 team versus forcing correctness for Javascript developers), but when I first started using node, I thought that it existed to serve the same purpose as it existed in Perl's use strict [1]: to enforce correct behavior and warn (stop) you when you're being sloppy with your code to the point where your code isn't doing what you think it is. It would have been a great blunder if a production release of Perl had bundled modules that didn't pass the strictness test. Likewise, anyone contributing a third party module on CPAN was also expected to write use strict compliant code, or at least any module writer who wanted to be taken seriously.

If V8's --use_strict isn't what node developers want to use for this purpose, I certainly hope that something similar to Perl's use strict is created by the node team to serve a similar purpose. While you're at it, copying perl -cw to validate a program without actually running it would be useful in node as well.

[1] http://perldoc.perl.org/strict.html

@rlidwka
Copy link

rlidwka commented Feb 14, 2015

If V8's --use_strict isn't what node developers want to use for this purpose

No, --use_strict is supposed to be only used internally in V8 tests. See #7479.

If you want to get warnings about "sloppy" code, you can use linters like jshint, eslint, etc.

misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 18, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. This change adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled, and fixes the built-in modules that are
currently broken.

Fixes nodejs#9187.
misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 18, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. This change adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled, and fixes the built-in modules that are
currently broken.

Fixes nodejs#9187.
@misterdjules
Copy link

@Tresdin Thanks for the information. Like several others pointed out already here and in #7479 , passing --use-strict to node, or more generally enabling "use strict"; for all code loaded in a given runtime is not necessarily a good idea, since not all code available complies to the rules of the strict mode.

Nevertheless, it seems that we can't really deprecate --use-strict, at least for the short term future. Always breaking users who use this flag would also generate a lot of questions and confusion. Fixing the code that breaks seems to be more effective than pointing to a FAQ or some documentation. For these reasons, I put together a PR that fixes some of node's built-in modules that currently fail to load with --use-strict.

Actually, if we decide to go down that path, I'm wondering if we should also add "use strict"; in every module file.

What do you think?

@clintdw
Copy link

clintdw commented Feb 18, 2015

I put together a PR that fixes some of node's built-in modules that currently fail to load with --use-strict.

Excellent.

Actually, if we decide to go down that path, I'm wondering if we should also add "use strict"; in every module file.

@Tresdin this sounds like a great idea, otherwise variations of bugs like this will appear wherever a built-in module isn't compliant.

@cjihrig
Copy link

cjihrig commented Feb 18, 2015

@misterdjules your PR can't be merged. What if we included 'use strict'; at the top of all the core files? It worked fine in io.js.

@misterdjules
Copy link

@cjihrig What do you mean by "can't be merged"? Yes, adding 'use strict'; at the top of all core files is also something that I suggested. It would not necessarily remove the need for a test though.

@cjihrig
Copy link

cjihrig commented Feb 18, 2015

Sorry, I just meant that there are merge conflicts.

@misterdjules
Copy link

@cjihrig It applies cleanly with curl https://github.com/joyent/node/pull/9237.patch | git am -:

➜  v0.12 git:(v0.12) ✗ git remote -v | grep upstream
upstream    git@github.com:joyent/node.git (fetch)
upstream    git@github.com:joyent/node.git (push)
➜  v0.12 git:(v0.12) ✗ git fetch upstream
➜  v0.12 git:(v0.12) ✗ git rev-parse --short HEAD
b3aa876
➜  v0.12 git:(v0.12) ✗ git rev-parse --short upstream/v0.12
b3aa876
➜  v0.12 git:(v0.12) ✗ curl https://github.com/joyent/node/pull/9237.patch | git am -
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7223    0  7223    0     0   6367      0 --:--:--  0:00:01 --:--:--  6369
Applying: src: fix builtin modules failing with --use-strict
➜  v0.12 git:(v0.12) ✗

Am I missing something?

@cjihrig
Copy link

cjihrig commented Feb 18, 2015

@misterdjules I meant the grey "We can’t automatically merge this pull request." on #9237. Is it showing up green for you?

@misterdjules
Copy link

@cjihrig Thank you, and sorry for not realizing that earlier :) I usually do not check the "merge section" in any PR because we don't use the merge button. I don't know why GitHub says it cannot be merged, I'll investigate further.

@lewispham
Copy link
Author

@misterdjules I think --use-strict should be globally applied by default. And Node need to provide a --non-strict flag instead. Don't be afraid of this decision. ECMAScript guys really want to do the same thing, but they can't. Just because it's too late. This is not true with the 5 years old Nodejs.

@rlidwka
Copy link

rlidwka commented Feb 18, 2015

I think --use-strict should be globally applied by default. And Node need to provide a --non-strict flag instead. Don't be afraid of this decision.

Sure. If we add a way to opt-out of it for each individual module (i.e. "use no strict" flag). But until then strict mode should never be the default.

misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 21, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. This change adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled, and fixes the built-in modules that are
currently broken.

Fixes nodejs#9187.
misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 23, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. In addition to that, console.trace
throws because it uses arguments.callee.

This change fixes these issues and adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled.

Please note that this change does not fix all issues with built-in
modules' code running with --use-strict. It is very likely that some
code in the built-in modules still fails when passing this flag.

However, fixing all code would require us to enable strict mode by
default in all builtins modules, which would certainly break existing
applications.

Fixes nodejs#9187.
misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 27, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. In addition to that, console.trace
throws because it uses arguments.callee.

This change fixes these issues and adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled.

Please note that this change does not fix all issues with built-in
modules' code running with --use-strict. It is very likely that some
code in the built-in modules still fails when passing this flag.

However, fixing all code would require us to enable strict mode by
default in all builtins modules, which would certainly break existing
applications.

Fixes nodejs#9187.
misterdjules pushed a commit to misterdjules/node that referenced this issue Feb 28, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. In addition to that, console.trace
throws because it uses arguments.callee.

This change fixes these issues and adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled.

Please note that this change does not fix all issues with built-in
modules' code running with --use-strict. It is very likely that some
code in the built-in modules still fails when passing this flag.

However, fixing all code would require us to enable strict mode by
default in all builtins modules, which would certainly break existing
applications.

Fixes nodejs#9187.

PR: nodejs#9237
PR-URL: nodejs#9237
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
misterdjules pushed a commit that referenced this issue Feb 28, 2015
Currently, lib/_tls_legacy.js and lib/crypto.js cannot be loaded when
--use-strict is passed to node. In addition to that, console.trace
throws because it uses arguments.callee.

This change fixes these issues and adds a test that makes sure
every external built-in module can be loaded with require when
--use-strict is enabled.

Please note that this change does not fix all issues with built-in
modules' code running with --use-strict. It is very likely that some
code in the built-in modules still fails when passing this flag.

However, fixing all code would require us to enable strict mode by
default in all builtins modules, which would certainly break existing
applications.

Fixes #9187.

PR: #9237
PR-URL: #9237
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@misterdjules
Copy link

Fixed in b233131, thank you!

@misterdjules
Copy link

Also, a follow-up commit that enables strict mode in all built-in modules has landed in master with ef43443.

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

No branches or pull requests

7 participants