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

Re-organize types module and remove logic from sys module #1027

Open
kjvalencik opened this issue Apr 1, 2024 · 0 comments
Open

Re-organize types module and remove logic from sys module #1027

kjvalencik opened this issue Apr 1, 2024 · 0 comments

Comments

@kjvalencik
Copy link
Member

The sys module was moved from a neon-runtime crate when the C++/NAN backend was removed. Originally it made sense to have logic there in order to abstract over the backend. However, now that there is a single backend it's no longer necessary to restrict calls to Node-API to this module. In fact, the extractor PR makes heavy use of sys functions for performance.

A cleaner and easier to follow model is a module for each JavaScript type.

neon
└── types_impl
    ├── buffer
    │   └── mod.rs
    ├── number
    │   └── mod.rs
    └── string
        └── mod.rs

In this structure, we would move TryFromJs implementations. In many cases, we could de-duplicate code by making fn value(..) implementations delegate to try_from_js. For example, JsString::value:

impl JsString {
    pub fn value<'cx, C>(&self, cx: &mut C) -> String
    where
        C: Context<'cx>,
    {
        String::try_from_js(self.upcast())?.unwrap()
    }
}
@kjvalencik kjvalencik changed the title Re-orgnize types module and remove logic from sys module Re-organize types module and remove logic from sys module Apr 3, 2024
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

No branches or pull requests

1 participant