Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Ban optional chaining delete (and anything else that may be missing?) #40

Closed
littledan opened this issue Nov 18, 2017 · 9 comments
Closed

Comments

@littledan
Copy link
Member

Due to the evidence from CoffeeScript, I thought we agreed to remove all constructs from optional chaining except for x.?y, x?.[y] and x?.(y). However, in this thread, @claudepache mentioned that optional chaining deletion is not yet banned. We should make it an early error in the draft spec, shouldn't we? Are there any other constructs that are currently allowed besides those three? The ambiguity here is delaying proper implementation in Babel.

@claudepache
Copy link
Collaborator

See Issue #14. I don’t see the point of forbidding optional deletion.

Although I understand you could conflate this case with optional write, it is qualitatively different in two ways:

  • human-wise, its semantics is unambiguous;
  • spec-wise, it requires no work to have it, literally.

About other potential constructs, I think they are properly handled:

  • some constructs are not understood by the grammar (thus a plain syntax error), e.g. new a?.b();
  • constructs that trigger write access, like a?.b++, are early errors thanks to the IsValidSimpleAssignmentTarget static semantics rule.

@littledan
Copy link
Member Author

I don't see anyone else supporting deletion in that thread. In the coffeescript data, I think there was only a single use of optional deletion. Would this be worth reconsidering?

@claudepache
Copy link
Collaborator

Do you have a strong reason for forbidding optional deletion?

Optional deletion has very few practical use cases; but forbidding it has probably no practical utility either. In those sort of edge cases, I think it is better to just try to be consistent with precedents, and see if that leads to something problematic or not. If unproblematic, just leave it as is.

The runtime semantics of delete is: When it receives

  1. a non-Reference or an unresolvable Reference: do nothing (and pretend to have succeeded);
  2. a resolvable Reference: attempt to delete it (that would throw an ReferenceError in strict mode if the Reference is not deletable).

As currently specced, delete (null)?.x falls in case 1 ((null)?.x evaluates to undefined, not to a reference that resolves to undefined).

The final outcome is exactly what a human would intuit. So, why forbid it?

If we had to redesign ECMAScript from scratch, maybe that case 1 would throw an error, and in that case an early error for optional delete would be indicated.

@claudepache
Copy link
Collaborator

Hi,

I have added in the explainer my justification for supporting optional deletion: f1a9945. Please check, and tell me if there is something to be amended and/or if you disagree.

@bcoe
Copy link
Contributor

bcoe commented Jul 1, 2019

I'd like to make a case for the delete operator, here's an example I can imagine coming up (in the land of writing client libraries, which is my life these days):

function request (url, opts) {
  // if options.headers is provided, remove the "If-Modified-Since".
  // such that we always request a new copy of the document.
  delete opts?.headers?.['If-Modified-Since']
}

@kaizhu256
Copy link

being nit-picky, but the optional-delete use-case for headers is a bad-example. the correct design-pattern is to normalize http-requests at integration-level (and we usually assume headers are included in the normalization):

function middlewareNormalize(req, res, nextMiddleware) {
/*
 * this middleware will normalize the <req> and <res> objects
 */
    req.headers = req.headers || {};
    req.queryDict = require("url").parse(req.url, true).query;
    ...
    nextMiddleware();
}

createServer({
    middlewareList:[
        middlewareNormalizeRequest,
        // rest of middlewares (and helper-functions) assume req.headers has been normalized
        middlewareValidate,
        ...
    ]
});

the same goes for client-requests -- headers are typically normalized at integration-level, and just assumed to exist:

function ajax(method, url, headers, data, callback) {
    var xhr;
    xhr = new XMLHttpRequest();
    // normalize headers once, and from now on, assume its just there
    xhr.headers = headers || {};
    ....
}

@bcoe
Copy link
Contributor

bcoe commented Jul 1, 2019

@kaizhu256 I went overboard trying to create a real-world example; I mainly mean to say that I can imagine a scenario where an API accepts undefined | object, and wants to clean keys off the object if passed into the constructor.

@MatthiasKunnen
Copy link

I've needed to use dynamice delete to remove fields before using Object.assign though in case of cleaning an object a recursive function without optional chaining will do the trick.

I personally would favor keeping optional delete if it does not impede progress on the rest of the proposal.

@claudepache
Copy link
Collaborator

Now that we are at Stage 3, we can consider that the semantics are fixed (except for the relatively minor issue of private field access, see #28). In particular, optional deletion is not banned.

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

No branches or pull requests

5 participants