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

'use strict' and eval #37

Closed
daviferreira opened this issue May 27, 2015 · 3 comments · Fixed by #38
Closed

'use strict' and eval #37

daviferreira opened this issue May 27, 2015 · 3 comments · Fixed by #38
Assignees
Labels

Comments

@daviferreira
Copy link
Contributor

I'm having this specific problem using classNames with webpack devtool option set to eval. It only happens on Chrome and when the console is not open. The issue is calling classNames inside the classNames function with the 'use strict' statement inside the function (https://github.com/JedWatson/classnames/blob/master/index.js#L22), this will cause a ReferenceError.

See:

webpack/webpack#417

Explanation:

ftlabs/fastclick#270

I would be happy to submit a PR with a umd wrapper if you guys are ok with it :)

Thanks for the great lib!

@dcousens
Copy link
Collaborator

@daviferreira could we just avoid the use of use strict?
We can enforce most of the structuring properties of strictness through a linter.

@dcousens dcousens self-assigned this May 27, 2015
@JedWatson
Copy link
Owner

I think we could take use strict out if it's causing trouble, or @daviferreira am happy to review your wrapper too if that solves the problem.

From what I understand, modern javascript engines are able to optimise certain code paths if they know a function is in strict mode, this may or may not be a major issue / benefit for classNames - unfortunately I'm not familiar enough with the inner workings to say for sure...

@dcousens
Copy link
Collaborator

@JedWatson this is apparently true, however I'd prefer to see some benchmarks before we just assume.

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

Successfully merging a pull request may close this issue.

3 participants