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

doc: Adding 'use strict' to ES6 examples #6380

Closed

Conversation

gh-amistry
Copy link

Checklist
  • [x ] documentation is changed or added
  • [x ] the commit message follows commit guidelines
Affected core subsystem(s)

doc: Adding 'use strict' to ES6 examples

Description of change

The events documentation provides examples for both traditional and ES6
syntax, however strict mode needs to be enabled in order for the ES6
examples to work without modification. I added 'use strict' to the full
examples (not snippets) and changed the preceding text in the first
example.

The events documentation provides examples for both traditional and ES6
syntax, however strict mode needs to be enabled in order for the ES6
examples to work without modification. I added 'use strict' to the full
examples (not snippets) and changed the preceding text in the first
example.
@addaleax addaleax added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Apr 25, 2016
@addaleax
Copy link
Member

Btw, this will work without strict mode in Node.js v6, which is scheduled to be released tomorrow.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 25, 2016

I'm not opposed to recommending strict mode in the code samples. Maybe the piece about it being required should target the v5.x branch though.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 25, 2016

I don't think we need this at this point, it makes the documentation less convenient and harder to understand. Also, these examples are not supposed to be executed as stand-alone files and do depend on the context.

Also, as @addaleax mentioned, this is going to work without the strict mode from tomorrow.

@silverwind
Copy link
Contributor

this is going to work out without the strict mode from tomorrow.

Does that mean strict mode will be the default mode from then on, or is that further off?

@gh-amistry
Copy link
Author

gh-amistry commented Apr 25, 2016

Thanks all. If the requirement for strict mode is going away in v6.x, I don't believe there's a need to update the documentation in this area. However, as @cjihrig mentioned, for v5.x, and other versions that need strict mode enabled, I still think it's a good idea. The ES6 examples may not have been intended to run as stand-alone files, but some of the examples are complete and will run if they are just copied and pasted (and using strict mode). It's for these examples that I think the update makes sense.

@jasnell
Copy link
Member

jasnell commented Apr 25, 2016

folks from the @nodejs/v8 team can likely clarify but the 'use strict'; is not going away, there are just quite a few of the newer ES6 features that can now be used without it.

@benjamingr
Copy link
Member

benjamingr commented Apr 26, 2016

I'm -1 on this, I think it adds more lines of code for no great reason. Maybe I'd mention this in the docs once at some point.

With modules, non-strict mode is going away for good hopefully (from mainstream code).

@jeisinger
Copy link
Contributor

The requirement to use strict mode for certain ES6 features comes from the spec, so it's likely going to stay that way.

@benjamingr
Copy link
Member

@jeisinger name such a feature.

@JacksonTian
Copy link
Contributor

-1. Class can be used without 'use strict' from v8 4.9.

@jeisinger
Copy link
Contributor

you don't get ES6 scoping in sloppy mode for example. In general, there's a long list of things that work for web compat in sloppy mode, and require 'use strict' if you actually want ES6 behavior.

the `util.inherits()` method. It is, however, possible to use ES6 classes as
well:
the `util.inherits()` method. It is, however, possible when using strict mode,
to use ES6 classes as well:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect, ES6 classes work without strict mode now.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 27, 2016

I think I covered all the changes here. At every place this PR changes, use strict either not required, or both not required and does nothing.

-1 from me.

This helped me find a mistype in the docs, though =).

@benjamingr
Copy link
Member

@jeisinger great, if there are so many then please name one in the docs :)

@jeisinger
Copy link
Contributor

@benjamingr
Copy link
Member

@jeisinger I'm well aware of what strict mode is and the impact it has (thanks though), it is first and foremost important since it provides lexical scoping which enables things like SES. It also has different primitive behavior (primitive as context doesn't get unboxed), eval and so on.

My point is that none of these examples apply in the docs, and since ES2015 modules are right around the corner and everything there runs in strict mode anyway I don't see the point in cluttering the docs like this.

@jeisinger
Copy link
Contributor

Yeah, sorry if I was unclear. I wasn't commenting on this CL, but on the question whether the semantics of strict more are going to change or not. And as far as they're part of the spec, they're not.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 27, 2016

Is there a reason to keep this open?

@benjamingr
Copy link
Member

No, I think we have a reasonable number of objections and won't reach consensus. Closing.

@benjamingr benjamingr closed this Apr 27, 2016
@gh-amistry
Copy link
Author

gh-amistry commented Apr 27, 2016

@ChALkeR Glad I could help find the typo at least! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants