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

Provide stricter types and restructure concerning types. #18

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

iddm
Copy link
Collaborator

@iddm iddm commented Apr 24, 2023

Additionally:

  1. Removes the unnecessary V8 prefixes for many structs and functions. It is unnecessary since all such types are already scoped within the v8-rs crate and even within the v8 module, which already means (twice) that the type is a V8 type.
  2. Provides a stricter type Value, which is an enumeration of some other possible values of JavaScript. This allows to have a clear distinction between the types and allows for a much easier, compile-time checked pattern matching.
  3. Renames some functions with the "new_" prefix to have the "create_" prefix instead. The "new_" prefix should be used for creating an object of the same type.
  4. Documents everything.
  5. Moves all the types under the "types" module so that those don't mess with the more global namespace and a semantically grouped into a single entity with subentities.
  6. Instead of custom-provided conversion methods, the std::convert::TryFrom and std::convert::From are used. This allows always to have a meaningful error message, avoids if/else conditions in favour of the conversion with an early return, and all the other cases are covered with the pattern matching of the value type.
  7. Adds the Type enum for the LocalValueAny type, which is generic and may contain any value. Adding this method instead of is_X allows using a much more advanced match statement, which is also much more strict when it comes to covering all the possible types (by having no fall-through).
  8. Some other minor fixes.

@iddm iddm requested a review from MeirShpilraien April 24, 2023 10:24
@iddm
Copy link
Collaborator Author

iddm commented Apr 24, 2023

Should ideally fix #16. At least a part of this pull request does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant