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

refactor spec #109

Closed devsnek closed 3 years ago

devsnek commented 3 years ago
devsnek commented 3 years ago

@dandclark ptal

dandclark commented 3 years ago

Thanks @devsnek for this change, just one other issue I noticed. In https://github.com/tc39/proposal-import-assertions/pull/106 we ended up doing a type check on the assertion values, rejecting the promise if any of them are not strings. The is an entry of supportedAssertions check was also moved inside the loop, so that this always rejects regardless of whether maybeSupportedAssertion is a supported assertion:

import("./foo", { assert: { maybeSupportedAssertion: { isAString: false } });

Could you update your change to keep the current behavior of rejecting for non-strings? After that's done I think this is ready to merge. I really like this refactoring.

dandclark commented 3 years ago

Thanks for the change!