Consistent quoting for Module['canvas']. NFC #22876
Merged
+23
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
127 or the 150 references to Module['canvas'] were already quoted. In general this is how we handle properties of the Module object that we don't want closure to minify. Once I do the same for the ctx object we can remove the closure type annotation completely from shell.js.
Member
|
Do we want this to remain on Module? I don't remember a reason for users to access it after construction, but I might be wrong. If we just need it during construction then we could receive it from Module and then access it like |
Collaborator
Author
Thats kind of my long term plan yes. |
kripken
approved these changes
Nov 7, 2024
Member
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
I see, this sounds good as a step then.
uysalibov
pushed a commit
to uysalibov/emscripten
that referenced
this pull request
Nov 7, 2024
127 or the 150 references to Module['canvas'] were already quoted. In general this is how we handle properties of the Module object that we don't want closure to minify. Once I do the same for the ctx object we can remove the closure type annotation completely from shell.js.
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Nov 7, 2024
This was the last property explicitly listed in the closure type annotate for the `Module` object. This means that it was previously being preserved by closure via this mechanism, but now we preserve it via the normal quoting mechanism. I've been looking at removing `ctx` and `canvas` from the `Module` object, but this change is just about consistency. See emscripten-core#22876
uysalibov
pushed a commit
to uysalibov/emscripten
that referenced
this pull request
Nov 15, 2024
127 or the 150 references to Module['canvas'] were already quoted. In general this is how we handle properties of the Module object that we don't want closure to minify. Once I do the same for the ctx object we can remove the closure type annotation completely from shell.js.
sbc100
added a commit
to sbc100/emscripten
that referenced
this pull request
Nov 26, 2024
This was the last property explicitly listed in the closure type annotate for the `Module` object. This means that it was previously being preserved by closure via this mechanism, but now we preserve it via the normal quoting mechanism. I've been looking at removing `ctx` and `canvas` from the `Module` object, but this change is just about consistency. See emscripten-core#22876
sbc100
added a commit
that referenced
this pull request
Nov 26, 2024
This was the last property explicitly listed in the closure type annotate for the `Module` object. This means that it was previously being preserved by closure via this mechanism, but now we preserve it via the normal quoting mechanism. I've been looking at removing `ctx` and `canvas` from the `Module` object, but this change is just about consistency. See #22876
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
127 or the 150 references to Module['canvas'] were already quoted. In general this is how we handle properties of the Module object that we don't want closure to minify. Once I do the same for the ctx object we can remove the closure type annotation completely from shell.js.