Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Switch default modules to es6 #62740

Closed
wants to merge 24 commits into from

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Dec 13, 2021

Motivation, benefits

  • TL;DR:
    • consistent in NodeJS and in browsers
    • module isolation
    • well defined exports/imports
    • dependencies resolution
    • dependencies detected during parsing, not during run
    • async dynamic imports awaitable
  • Why Modules
  • Supported everywhere for many years, everywhere where WASM is able to run.

Challenges, downsides

  • CommonJS module could be wrapped inside simple ES6 script. But not the other way around.
    • ES6 modules could be re-packaged by standard tools like webpack, rollup back to CJS or UMD.
    • libraries using another ES6 modules have to be also ES6 module (unless they re-pack with tools above).
  • Node.js 12 doesn't support it, but is end-of-life by 2022-04-30
  • After this PR will would still have way how to produce CommonJS, but is it worth supporting it ?

Changes in this PR

  • switched dotnet.js to be ES6 module by making <WasmEnableES6> default true
  • updated all samples
  • updated functional tests
  • updated debugger tests
  • updated test-main to
  • EXPORT_ES6=1 should not be in the emcc-link.rsp because it could not be removed by workload re-link when <WasmEnableES6> == false.

@pavelsavara pavelsavara added NO-REVIEW Experimental/testing PR, do NOT review it arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript labels Dec 13, 2021
@pavelsavara pavelsavara added this to the 7.0.0 milestone Dec 13, 2021
@ghost
Copy link

ghost commented Dec 13, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Just testing on CI right now.
On top of #62712 and #62292

Author: pavelsavara
Assignees: -
Labels:

NO REVIEW, arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 7.0.0

@pavelsavara
Copy link
Member Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara force-pushed the wasm_switch_to_es6 branch 2 times, most recently from ac777b2 to dd675ef Compare January 13, 2022 12:13
@pavelsavara
Copy link
Member Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara removed the NO-REVIEW Experimental/testing PR, do NOT review it label Jan 13, 2022
@pavelsavara pavelsavara changed the title [DRAFT][wasm] Switch default modules to es6 [wasm] Switch default modules to es6 Jan 13, 2022
@pavelsavara
Copy link
Member Author

/azp run runtime-manual

@maraf
Copy link
Member

maraf commented Jan 21, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts:
#	src/mono/wasm/runtime/CMakeLists.txt
#	src/mono/wasm/wasm.proj
@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts:
#	src/mono/wasm/test-main.js
@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented Mar 9, 2022

Draft Pull Request was automatically closed for inactivity. Please let us know if you'd like to reopen it.

@ghost ghost closed this Mar 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2022
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants