Fix top-level exports corner cases. #128
Conversation
This is necessary for exported Scala classes to be instantiable with `new ExportedClass()`.
There was a problem hiding this comment.
I left a few questions, but overall it looks good to me 👍
Having some comments (something like commit message on 208202a) at transformTopLevelExport would be helpful
| /* Usually redundant, but necessary if the static field is never | ||
| * explicitly set and keeps its default (zero) value instead. |
There was a problem hiding this comment.
Why it's necessary in that case? For initializing JS mirror to be its default (zero)?
| case d: TopLevelJSClassExportDef => genDelayedTopLevelExport(d.exportName) | ||
| case d: TopLevelModuleExportDef => genDelayedTopLevelExport(d.exportName) |
There was a problem hiding this comment.
Instead of defining Global in Wasm, genTopLevelExportSetter eventually generate declaration (and setter) in JS via buildJSFileContent.
| case d: TopLevelJSClassExportDef => genDelayedTopLevelExport(d.exportName) | ||
| case d: TopLevelModuleExportDef => genDelayedTopLevelExport(d.exportName) | ||
| case d: TopLevelMethodExportDef => transformTopLevelMethodExportDef(d) | ||
| case d: TopLevelFieldExportDef => transformTopLevelFieldExportDef(d) |
There was a problem hiding this comment.
So, we will have two different global states for TopLevelFieldExportDef
- Global in Wasm
- and JS (by buildJSFileContent?)
There was a problem hiding this comment.
Ah, so that's why we have a mirror, and when global is updated, it also updates the content of JS here in genAssign's case SelectStatic.
https://github.com/tanishiking/scala-wasm/pull/128/files#diff-21791d7e9413ad6e99a12c7089bf6f6ebbb9143ee2ee4c35ca1e831f80531e4eR463-R475
The previous strategy for top-level export could not lead to any workable support of mutable vars. We now follow a strategy similar to what the JS backend uses in its NoModule configuration. Previously, we compiled top-level exports as Wasm global exports. We then had a postprocessing step in the loader to extract the `.value` of the `WAGlobal` instances. This correctly captured the exported value right after the start function has finished. However, it could not update the exported value after a mutable static field was reassigned. Now, we turn everything everything around. Instead of the JS loader and wrapper *pulling* values from Wasm, the Wasm code *pushes* updates to top-level exports to JavaScript. That means we declare setter functions from JavaScript, and that we (counter-intuitively) *import* those setters into Wasm. Wasm calls the setters in the start function for all top-level exports, and again when assigning static fields that are exported.
sjrd
force-pushed
the
fix-top-level-exports
branch
from
1a8dad5
to
4fdbed2
Compare
|
I added comments as suggested. |
| /* Usually redundant, but necessary if the static field is never | ||
| * explicitly set and keeps its default (zero) value instead. In that | ||
| * case this initial call is required to publish that zero value (as | ||
| * opposed to the default `undefined` value of the JS `let`). |
No description provided.