search

tc39 / proposal-import-attributes

Proposal for syntax to import ES modules with assertions
https://tc39.es/proposal-import-attributes/
Apache License 2.0
569 stars 32 forks source link

Ensure Object type in dynamic import() Runtime Semantics #108

Closed dandclark closed 3 years ago

dandclark commented 3 years ago

In the Runtime Semantics for the version of dynamic import() that takes an assertions argument, the second argRef and the assertionsObj are not guaranteed to be Objects. e.g:

import("./foo", 42);
import("./foo", { assert: 42 });

But, Get() and EnumerableOwnPropertyNames both assert that the argument to them is an Object.

This change adds checks that these are of Type Object, and reject the promise with a TypeError if either is not.

assertionsObj is still allowed to be undefined. This will be useful if we use the options bag for other things such as evaluator attributes, where a missing assertionsObj shouldn't result in failure:

import('./foo", { with: { transformA: "value" } });
bmeck commented 3 years ago

No concerns if you throw. Problems around polluting the value via prototype aren't possible.

On Thu, Apr 29, 2021, 6:19 PM Dan Clark @.***> wrote:

@.**** commented on this pull request.

In spec.html https://github.com/tc39/proposal-import-assertions/pull/108#discussion_r623473931 :

    1. If Type(arg) is not Object, then
    1. Perform ! Call(promiseCapability.[[Reject]], undefined, « a newly created TypeError object »).

This was discussed in April's meeting and it seems like there was pretty broad agreement that throwing for non-Object, rather than converting ToObject, is the direction we want to go. This matches what ECMA 402 is doing with GetOption() (link https://github.com/tc39/notes/blob/master/meetings/2020-09/sept-22.md#getoption-in-ecma-262 ).

@bmeck https://github.com/bmeck you mentioned that you had varied opinions on whether to reject or not based on whether we pull off the prototype (or something along those lines -- I didn't quite follow at the time). You said that you wanted to follow up offline, so I wanted to give you a chance to chime in here in case there was any concern you wanted to raise before I go ahead and land this PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-import-assertions/pull/108#discussion_r623473931, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABZJI33CYAGD3SRAVVG353TLHSRJANCNFSM4V77TAQA .