Skip to content

use strict mode should be at the root of the module #32

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

Merged
merged 3 commits into from
Jul 1, 2015

Conversation

greelgorke
Copy link
Contributor

because a module code is basically a function body

solves #31

because a module code is basically a function body
@greelgorke
Copy link
Contributor Author

i see the build is failing because of the eslint rule no-extra-strict

i would suggest, that we should remove the struct pramas from the inner functions, because the module-wide one should cover it.

the rule itself is depricated btw.

@ljharb
Copy link
Collaborator

ljharb commented Jul 1, 2015

The concern is that this file is occasionally just ran in the global scope - not always as a module. That's the reason I only strict moded the individual functions.

I don't understand why this should make a difference though, since none of the functionality of the file relies on either strict or sloppy behavior?

@greelgorke
Copy link
Contributor Author

its run in global scope? it is a a node module and supposed to be used as such right? node.js modules are basicaly function bodies, they should be wrapped into such by the loaders. if someone uses it without the loader or any kind of wrapper, than it's basically a misuse case.

the issue as in the referenced one, is that some loaders are using eval (like webpack) for some tasks. in webpacks case its a source maps support feature. and somehow eval and lack of the module-wide pragma is causing the ReferenceErrors.

i don't know why this is an issue. here is a related issue https://code.google.com/p/chromium/issues/detail?id=491536 that indicates it to be a bug in v8, so this PR would be mor of a workaround of this bug.

for solving the problem we could wrap the module in a function explicitly so the global-scope usecase will be covered. would this be a solution?

@ljharb
Copy link
Collaborator

ljharb commented Jul 1, 2015

Yeah, that's a fair point. I'm OK not supporting non-browserify-ish use cases.

I'll consider this a breaking change, just in case, but I think it's a good one.

ljharb added a commit that referenced this pull request Jul 1, 2015
use strict mode should be at the root of the module
@ljharb ljharb merged commit 273ab44 into justmoon:master Jul 1, 2015
@greelgorke
Copy link
Contributor Author

thank you :)

ljharb added a commit to ljharb/node-extend that referenced this pull request Jul 19, 2018

Verified

This commit was signed with the committer’s verified signature.
ljharb Jordan Harband
This reverts commit 273ab44, reversing
changes made to 2f3efa2.
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

Successfully merging this pull request may close these issues.

None yet

2 participants