-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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] updated samples for ES6 and CJS #62292
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue Detailswork in progress
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0c25eea
to
e5a6072
Compare
7913737
to
bfa824e
Compare
bffe9eb
to
55a2562
Compare
55a2562
to
11f277e
Compare
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
a87fd60
to
bf2feca
Compare
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
bf2feca
to
d6b376a
Compare
/azp run runtime-manual |
d723b9d
to
3f71bce
Compare
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures are unrelated #62964 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
or( | ||
eq(variables['librariesContainsChange'], true), | ||
eq(variables['monoContainsChange'], true), | ||
eq(variables['isFullMatrix'], true)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we want these conditions anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, there is no other place which would run on NodeJS platform every time. Also this is runtime-manual
pipeline. So it's already run with intention of getting more. I'm prepared to change it if it doesn't make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We agreed with Radek to put it back and make it consistent with other ones in the same file.
@steveisok eq(variables['isManualOrIsNotPR'], true),
is this always true in this file ? Was is the idea behind it ? Or is that just copy&paste like in my case ?
src/mono/sample/wasm/browser-es6/Wasm.Browser.ES6.Sample.csproj
Outdated
Show resolved
Hide resolved
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-manual |
Azure Pipelines successfully started running 1 pipeline(s). |
Modularized API and ES6
Reworked samples which demonstrate how to use modularized API and ES6
To generate ES6 or CommonJS version of
dotnet.js
package.json
with"type":"module"
to import es6 script with '.js' extensionIn-tree relink of ES6 version implemented as MsBuild hack right now and would be fixed as part of #61294
ES6 Samples
Samples demonstrate the new API createDotnetRuntime()
Minimal browser sample
Still trivial sample
CommonJS
NodeJS Sample
Contributes to #47336
Contributes to #41861
Fixes #61083