Throw when doing import defer * of a module with no exports
#34
Conversation
| @@ -840,6 +843,7 @@ contributors: Nicolò Ribaudo | |||
| 1. <del>For each String _required_ of _module_.[[RequestedModules]], do</del> | |||
| 1. <ins>For each ModuleRequest Record _required_ of _module_.[[RequestedModules]], do</ins> | |||
| 1. Let _requiredModule_ be GetImportedModule(_module_, _required_<ins>.[[Specifier]]</ins>). | |||
| 1. <ins>If _required_.[[Phase]] is ~defer~ and _requiredModule_.GetExportedNames() is empty, throw a new *SyntaxError*.</ins> | |||
There was a problem hiding this comment.
Note: other linking errors due to missing exports are also syntax errors.
There was a problem hiding this comment.
Typically named exports syntax errors apply to both the consumer and producer of a module, since on either side it is a syntax error not to define the correct export name.
In the case of defer, these are different though - the consumer has the intent of deferring execution, so it is not a syntax error if they don't realise the module has no exports. But from the perspective of the producer of the module being deferred it could be interpreted as a syntax error that they did not define export names to be deferred.
I do think perhaps the consumer use case wins here, in that a syntax error would be quite confusing to debug as a consumer using defer, while an explicit type error could make it clear that the "type of the module objet does not support defer".
There was a problem hiding this comment.
why would it not be a valid use case to defer import of a side-effecting zero-export module?
There was a problem hiding this comment.
It may be a valid use case, but the converse argument is yours in #19 (comment).
There was a problem hiding this comment.
rofl indeed so.
is there a way to initiate the evaluation that's not accessing something on the namespace object? (since toStringTag and __esModule would be things we DONT want to trigger it)
There was a problem hiding this comment.
🤔
maybe Object.keys(deferred) and other forms of reflection could trigger it, via the MOP hooks?
There was a problem hiding this comment.
I'm not actually sure if this would work or not, perhaps the iteration is not execution triggering?
There was a problem hiding this comment.
Right now evaluation only happens during [[Get]]. It's possible to add it to more MOPs, but it makes it more difficult to teach why passing a ns object to some functions triggers evaluation and to other it doesn't.
Object.keys right now triggers evaluation only of non-empty namespaces, because it only calls [[Get]] if there are any own properties.
There was a problem hiding this comment.
Object.keys doesn't call [[Get]] at all, i think? Just Object.values/entries do - Object.keys calls [[OwnPropertyKeys]] and [[GetOwnProperty]]. I would expect all three of those to trigger evaluation.
There was a problem hiding this comment.
Yes, that would make sense to me, and I agree we shouldn't extend the behaviours unexpectedly here.
After some further thought I must also admit I'm coming around to the idea of permitting empty namespaces instead of throwing. Because it's an edge case, allowing it to work may be better for users than not.
|
Thanks for adding this, I have a weak preference for this form. Seems sensible to leave this one open to discussion? |
|
I'm against. It hurts consistency. Edge cases, even when they're pointless and advisable not to write, should behave the same, as much as possible, as if there were no sharp edge. It simplifies semantics, tools relying on semantics, spec size and teaching. It also saves special cases in code generation scenarios. |
@guybedford Is this fix better than #33?
Closes #19