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

Editorial: disambiguate with statement's status #2441

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

ddbeck
Copy link
Contributor

@ddbeck ddbeck commented Jun 23, 2021

Currently, the spec sends mixed signals to programmers about the with statement; this PR attempts to clear up the ambiguity by adding text that explicitly discourages the use of the with statement in new ECMAScript code.

This is an attempt to solve a specific problem: developers and technical writers are unclear about the status of the with statement. See this StackOverflow question or this discussion in MDN Web Docs's browser-compat-data project to get a taste. This makes it difficult to document on MDN Web Docs and elsewhere.

By forbidding with in strict mode code and cutting it off from many contemporary language features, the specification seems to imply that programmers shouldn't use with, but this is less explicit than other disfavored parts of the language (e.g., the note for the getYear method or the introduction to Annex B). Because there's no single way that the spec indicates non-preferred features, developers and technical writers have to make do with close readings and interpretations.

Ideally, the specification could resolve the question definitively. This PR's fix to the problem is text that speaks directly to programmers, discouraging them from using the with statement. This is my attempt to put into words what I believe the existing text implies to programmers, but I'm open to making revisions which more accurately captures the posture toward with statement.

Thanks for considering this change!

@rolivo3
Copy link

rolivo3 commented Jun 23, 2021

The message must be very clear if this characteristic is going to be deprecated or not. One thing is for it to be discouraged and the other more traumatic for it to be deprecated.

The specification should have a more formal definition of "deprecated " and "discouraged".

Removing a feature is very painful on high impact. Also, if a characteristic that has been considered stable (for me if it has reached stage 3 or 4 of the process) is deprecated, the meaning of stable is lost, can this be clarified? What is stable and what is not?

@michaelficarra
Copy link
Member

#2125 will be defining the term "legacy" for this purpose. However, applying the term "legacy" to a feature requires committee consensus, so this will have to be marked normative, not editorial.

@ljharb
Copy link
Member

ljharb commented Jun 23, 2021

It seems like this might be better marked as "Legacy", once #2125 lands (which would require consensus)

@ljharb
Copy link
Member

ljharb commented Jun 23, 2021

@michaelficarra i don't think it's normative just because it requires consensus.

@WebReflection
Copy link

There are no substitutes for the with statement, and the spec has Symbol.unscopables to deal with those cases too.

Discouraging with reasons looks like a good way forward, but "deprecating" it makes little sense.

I hope the obj.{ ... }; syntax, one day, will be back, otherwise we are all in Function('...') hands to bring back what with can do, as nothing else can.

@ljharb
Copy link
Member

ljharb commented Jun 28, 2021

deprecation (in a context where basically nothing can be removed) is the same as discouragement.

@ljharb ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21
@ddbeck
Copy link
Contributor Author

ddbeck commented Jun 29, 2021

Thanks for the comments here. I wasn't previously aware of #2125. "Legacy" seems like a great way to handle this sort of thing. When #2125 is merged, I'll be happy to update this PR to use the legacy attribute, though I'm not certain if my note should be retained or not.

applying the term "legacy" to a feature requires committee consensus

I'm new to contributing to the specification and the contributing docs don't seem to cover a case like this. What, if anything, should I do make it easier for the committee to consider this for consensus?

@michaelficarra
Copy link
Member

@ddbeck The typical process would be to find a champion, which is a representative of a TC39 member organisation who will present your topic to committee during one of the regular meetings. If you update the language here to use the term "legacy", as defined in #2125, I will present this topic as a needs-consensus PR for you at the next meeting. The case for with is not as obvious as the already agreed-upon applications of "legacy", but I do personally think it merits the term.

@ddbeck ddbeck force-pushed the note-with-statement branch from 7b39b7f to 55ce3c3 Compare July 2, 2021 21:41
@ddbeck
Copy link
Contributor Author

ddbeck commented Jul 2, 2021

@michaelficarra Excellent, thank you. I'd be delighted if you and the rest of the committee to take this up at the next meeting. I've updated the PR to use the term "legacy" and to set the legacy attribute for the clause (though this shouldn't really do anything until the other PR is merged). As soon as the other PR is merged, I'll rebase on top of that, too. Thank you again!

@ljharb ljharb added editorial change needs consensus This needs committee consensus before it can be eligible to be merged. labels Jul 2, 2021
@ddbeck ddbeck force-pushed the note-with-statement branch from 55ce3c3 to 91a3c84 Compare July 28, 2021 15:33
@ddbeck
Copy link
Contributor Author

ddbeck commented Jul 28, 2021

Since #2125 merged last week, I rebased my changes against the default branch and confirmed that this builds properly with the legacy attribute (since, as a first-time contributor, the GitHub workflows don't run).

I'll be happy to make any additional changes requested, as this proposal works its way through the process. Thanks again!

@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Aug 31, 2021
@michaelficarra
Copy link
Member

@ddbeck This was presented to committee today (by @ljharb) and achieved consensus. 🎉

spec.html Outdated Show resolved Hide resolved
@ddbeck ddbeck force-pushed the note-with-statement branch 2 times, most recently from 560948a to d53361d Compare September 6, 2021 15:36
spec.html Outdated Show resolved Hide resolved
@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Sep 10, 2021
@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Sep 16, 2021
@michaelficarra michaelficarra requested a review from a team September 16, 2021 01:36
@bakkot
Copy link
Contributor

bakkot commented Sep 16, 2021

We should put destructuring assignment inside of the xref element, so that it's not just a section number.

(Github isn't letting me comment directly on the PR for some reason.)

@michaelficarra
Copy link
Member

@bakkot done

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Sep 16, 2021
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
Co-authored-by: Michael Ficarra <mficarra@shapesecurity.com>
@ljharb ljharb force-pushed the note-with-statement branch from 627420a to f125a96 Compare September 16, 2021 05:18
@ljharb ljharb merged commit f125a96 into tc39:master Sep 16, 2021
@ddbeck ddbeck deleted the note-with-statement branch September 16, 2021 14:12
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
Co-authored-by: Michael Ficarra <mficarra@shapesecurity.com>
@ljharb

This comment was marked as resolved.

@ddbeck

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change has consensus This has committee consensus. ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants