[WASM] Enabled strict mode on all JS files

[Daniel-Genkin]

This PR adds the "use strict" annotation to all JS files (including dotnet.js via the emcc STRICT_JS flag).

Why is this needed?

This annotation signals to the JS engine that the file should be run in strict mode which throws errors/exceptions when some looser features of JS are used (for example using undeclared variables). This is also required to ease the eventual NodeJS support that I am working on adding to wasm.

What's changed

Basically every file ending with .js will have the "use strict" annotation on the first line. This is mostly done manually with the only exception being the dotnet.js file as it is automatically generated. For that I used the STRICT_JS emcc flag.

Other minor changes

• Some of the files in the runtime folder where a mix of tabs and spaces. I changed them to be all tabs (there were more tabs than spaces).
• There were a few unused variables that I removed and a few that I marked with comments

[ghost]

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This PR adds the "use strict" annotation to all JS files (including dotnet.js via the emcc STRICT_JS flag).

Why is this needed?

This annotation signals to the JS engine that the file should be run in strict mode which throws errors/exceptions when some looser features of JS are used (for example using undeclared variables). This is also required to ease the eventual NodeJS support that I am working on adding to wasm.

What's changed

Basically every file ending with .js will have the "use strict" annotation on the first line. This is mostly done manually with the only exception being the dotnet.js file as it is automatically generated. For that I used the STRICT_JS emcc flag.

Other minor changes

• Some of the files in the runtime folder where a mix of tabs and spaces. I changed them to be all tabs (there were more tabs than spaces).
• There were a few unused variables that I removed and a few that I marked with comments

Author:	Daniel-Genkin
Assignees:	-
Labels:	area-VM-meta-mono
Milestone:	-

[ghost]

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

Issue Details

---

This PR adds the "use strict" annotation to all JS files (including dotnet.js via the emcc STRICT_JS flag).

Why is this needed?

This annotation signals to the JS engine that the file should be run in strict mode which throws errors/exceptions when some looser features of JS are used (for example using undeclared variables). This is also required to ease the eventual NodeJS support that I am working on adding to wasm.

What's changed

Basically every file ending with .js will have the "use strict" annotation on the first line. This is mostly done manually with the only exception being the dotnet.js file as it is automatically generated. For that I used the STRICT_JS emcc flag.

Other minor changes

• Some of the files in the runtime folder where a mix of tabs and spaces. I changed them to be all tabs (there were more tabs than spaces).
• There were a few unused variables that I removed and a few that I marked with comments

Author:	Daniel-Genkin
Assignees:	-
Labels:	arch-wasm, area-VM-meta-mono
Milestone:	-

[pavelsavara]

Could you please change var to const everywhere where we do not re-assign or let otherwise ?
Thank you!

[Daniel-Genkin]

Could you please change var to const everywhere where we do not re-assign or let otherwise ?
Thank you!

Good idea! However it is kind of parallel to this PR, so I'll make a new PR for this after this one is completed.

[kg]

This generally seems fine, but I think mixing all these whitespace changes in might be a mistake. It clutters the git blame and makes it unclear what this PR actually did or didn't do.

[Daniel-Genkin]

@kg Should they be removed? I just converted all the files in runtime to use tabs instead of spaces for consistency since I was already making such syntax changes (see the other minor changes section in the description). Before this, some files had a mix of spaces and tabs.

[kg]

@kg Should they be removed? I just converted all the files in runtime to use tabs instead of spaces for consistency since I was already making such syntax changes (see the other minor changes section in the description). Before this, some files had a mix of spaces and tabs.

I don't know if we have a policy for that in /runtime. It usually makes diffs and blame harder to review, but github has a 'hide whitespace changes' option so maybe it's fine.

[kg]

Other than my previous comments I'm happy with this PR, thanks for doing it. I can't give it a green check yet though because if it landed it would block the marshaler PR, so I want to make sure that gets in first unless we decide not to land it at all

[Daniel-Genkin]

Other than my previous comments I'm happy with this PR, thanks for doing it. I can't give it a green check yet though because if it landed it would block the marshaler PR, so I want to make sure that gets in first unless we decide not to land it at all

Ok. Thanks for your review. That makes sense. Do you think the marshaler PR will be done sometime soon?

[kg]

Other than my previous comments I'm happy with this PR, thanks for doing it. I can't give it a green check yet though because if it landed it would block the marshaler PR, so I want to make sure that gets in first unless we decide not to land it at all

Ok. Thanks for your review. That makes sense. Do you think the marshaler PR will be done sometime soon?

I'm working on figuring that out as we speak. Sorry for delaying your work with this

[Daniel-Genkin]

I'm working on figuring that out as we speak. Sorry for delaying your work with this

Haha ok sounds good. I'll work on some other tasks for now then. Thanks again

[lewing]

I don't see any dotnet_support.js changes?

[Daniel-Genkin]

I don't see any dotnet_support.js changes?

I didn't see anything that wasn't compliant with strict mode. Do you see something? Primarily, I was looking for undeclared variables or other strict mode related errors.

[lewing]

Looking good, a couple of comments and one issue.

[Daniel-Genkin]

Note that this PR should be merged after #54281 to minimize merge conflicts and to avoid mixing them up.
Update: #54281 has been closed but this PR is still waiting on the marshaler one (#47640)

[marek-safar]

What is the plan with this PR?

[kg]

If we land it all the marshaler and invoke stuff will probably have to be rewritten to rebase it. Otherwise the PR looks great

[Daniel-Genkin]

Yes I left this pending while Katelyn works on the marshaler one. It has not been forgotten :)

[pavelsavara]

LGTM, what do you think @kg ?

[kg]

LGTM, what do you think @kg ?

My previous opinion (it's great) holds, but we should come to a firm decision w/Larry or other involved decision-makers that we're going to abandon the invoke and/or marshaler PRs before we land it, since that's what landing it implies.

[pavelsavara]

I understood Larry said that we are not going to land it in 6.0
Also I think it only implies we would have to eventually re-base it later, not necessarily abandon it.
And we are already also doing lot of other stuff in the codebase which has similar effect as this PR.
Let's not stop the world ;-) My 2c

[pavelsavara]

@kg @lewing I would like to merge it later today, please let me know if you have any pending concerns.

Too busy to look again

[pavelsavara]

Failure is #58812 and other wasm unrelated failures