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

runtime(js): use es6 class #1840

Merged
merged 2 commits into from
Mar 3, 2025
Merged

runtime(js): use es6 class #1840

merged 2 commits into from
Mar 3, 2025

Conversation

smorimoto
Copy link
Member

@smorimoto smorimoto commented Feb 23, 2025

This PR updates several JavaScript runtime components to use ES6 class syntax instead of the traditional prototype-based object model. This modernization improves code readability and maintainability while aligning with current JavaScript best practices.

Changes

  • Converted the classes to ES6 syntax
  • Updated class inheritance patterns to use the extends keyword
  • Replaced prototype method definitions with class methods
  • Maintained all existing functionality while improving code structure

Benefits

  • More readable and maintainable code structure
  • Better alignment with modern JavaScript development practices
  • Clearer class hierarchies through explicit extends relationships
  • Improved code organization with methods contained within class definitions

Documentation

  • Updated ECMASCRIPT.md to document ES6 class support

Testing

All existing tests continue to pass with the updated class implementations.


I recommend clicking this checkbox when reviewing on GitHub to improve readability:
image

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

runtime/js/marshal.js:740

  • [nitpick] Redeclaring the parameter 'pos' with 'var pos = pos;' is redundant and can be confusing; it is recommended to remove the redeclaration to use the parameter directly.
var pos = pos;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 11 changed files in this pull request and generated no comments.

Files not reviewed (6)
  • runtime/js/bigarray.js: Evaluated as low risk
  • runtime/js/nat.js: Evaluated as low risk
  • runtime/js/sync.js: Evaluated as low risk
  • runtime/js/fs_fake.js: Evaluated as low risk
  • runtime/js/int64.js: Evaluated as low risk
  • runtime/js/fs_node.js: Evaluated as low risk
@smorimoto smorimoto force-pushed the classes branch 2 times, most recently from c6eb838 to cf8186a Compare February 26, 2025 16:07
Signed-off-by: Sora Morimoto <sora@morimoto.io>
@smorimoto
Copy link
Member Author

Related PR: #1846

@hhugo hhugo merged commit 2fee5f0 into master Mar 3, 2025
30 of 31 checks passed
@hhugo hhugo deleted the classes branch March 3, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants