update spec #13
update spec #13
Conversation
devsnek
force-pushed
the
rebase
branch
3 times, most recently
from
9387a01 to
97eaab3
Compare
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-parse-json-module" aoid="ParseJSONModule"> | ||
| <h1>ParseJSONModule ( _source_ )</h1> | ||
| <p>The abstract operation ParseJSONModule takes a single argument _source_ which is a String representing the contents of a module.</p> | ||
| <h1>ParseJSONModule ( _sourceText_ , _realm_ , _hostDefined_ )</h1> |
There was a problem hiding this comment.
The call site doesn't specify arguments for either version of the signature.
If type is "json", then this algorithm must either invoke ParseJSONModule and return the resulting Completion Record, or throw an exception.
Is that intentional?
There was a problem hiding this comment.
The reference in "Runtime Semantics: HostResolveImportedModule".
There was a problem hiding this comment.
ah, I think so. the steps are host defined, so we mandate that they call it, not what they call it with.
There was a problem hiding this comment.
Ah, thanks! Ambiguity for the host-defined algorithms seems appropriate--I think what tripped me up is the difference in how we're acknowledging input and output for that operation. We don't mandate what implementations call the abstract operation with, but we do mandate what they do with the result. In a way, the function seems like the boundary for the normative text, so it's hard for me to classify the quoted statement as strictly normative or strictly informative.
Seems like I'm alone in that boat, though, so I'm happy to let it ride :)
| 1. Resume the context that is now on the top of the execution context stack as the running execution context. | ||
| 1. Return Completion(_result_). | ||
| </emu-alg> | ||
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-synthetic-module-record-set-synthetic-export"> | ||
| <h1>SetSyntheticExport ( _name_, _value_ )</h1> |
There was a problem hiding this comment.
Was the name change from SetSyntheticModuleExport deliberate? If so there are a couple other instances of SetSyntheticModuleExport that need to be updated on lines 94 and 116.
If we're going to rename this though, I'd also like to consider just SetExport. This aligns a bit better with the name of ResolveExport. I'm not sure that "Synthetic" needs to be part of the name since this is already a method on Synthetic Module Record.
Not a strong preference either way though.
There was a problem hiding this comment.
To be clear, I think the original name SetSyntheticModuleExport was also fine.
| @@ -131,112 +131,109 @@ <h1>CreateSyntheticModule ( _exportNames_, _evaluationSteps_, _realm_, _hostDefi | |||
| <emu-note type="editor">It seems we could set up the environment either here or in Instantiate(). I've chosen to do so in Instantiate() for symmetry with Source Text Module Records, but I don't think there's any actual requirement in that regard.</emu-note> | |||
There was a problem hiding this comment.
| <emu-note type="editor">It seems we could set up the environment either here or in Instantiate(). I've chosen to do so in Instantiate() for symmetry with Source Text Module Records, but I don't think there's any actual requirement in that regard.</emu-note> | |
| <emu-note type="editor">It seems we could set up the environment either here or in Link(). I've chosen to do so in Link() for symmetry with Source Text Module Records, but I don't think there's any actual requirement in that regard.</emu-note> |
| <h1>ParseJSONModule ( _source_ )</h1> | ||
| <p>The abstract operation ParseJSONModule takes a single argument _source_ which is a String representing the contents of a module.</p> | ||
| <h1>ParseJSONModule ( _sourceText_ , _realm_ , _hostDefined_ )</h1> | ||
| <p>The abstract operation ParseJSONModule takes arguments _sourceText_ (a String), _realm_ (a Realm Record), and _hostDefined_.</p> |
There was a problem hiding this comment.
Could we omit the _hostDefined_ parameter in ParseJSONModule and CreateDefaultExportSyntheticModule? Now that we're no longer using [[HostDefined]] to stash the default export value, it seems that we could just pass undefined when CreateDefaultExportSyntheticModule calls CreateSyntheticModule.
The _hostDefined_ parameter added in this PR give the host the power to stash something arbitrary in the JSON module's [[HostDefined]], independent of the value of the default export, and I'm not sure that's necessary. As far as I know there are no dependencies on JSON module's [[HostDefined]] other than the one that you've eliminated here.
No description provided.