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

Make modules public in http #655

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Sytten
Copy link
Collaborator

@Sytten Sytten commented Oct 28, 2024

Description of changes

Make relevant modules public on llrt_http so people can craft their own module

Checklist

  • Created unit tests in tests/unit and/or in Rust for my feature if needed
  • Ran make fix to format JS and apply Clippy auto fixes
  • Made sure my code didn't add any additional warnings: make check
  • Added relevant type info in types/ directory
  • Updated documentation if needed (API.md/README.md/Other)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Sytten
Copy link
Collaborator Author

Sytten commented Oct 28, 2024

This allows a user to create it's own module if they dont want http in the root

use rquickjs::module::{Declarations, Exports, ModuleDef};
use rquickjs::{Class, Ctx, Result};

pub struct HttpModule;

impl ModuleDef for HttpModule {
    fn declare(declare: &Declarations) -> Result<()> {
        declare.declare("fetch")?;
        declare.declare(stringify!(Request))?;
        declare.declare(stringify!(Response))?;
        declare.declare(stringify!(Headers))?;
        declare.declare("Blob")?;
        declare.declare(stringify!(File))?;
        declare.declare("default")?;

        Ok(())
    }

    fn evaluate<'js>(ctx: &Ctx<'js>, exports: &Exports<'js>) -> Result<()> {
        llrt_utils::module::export_default(ctx, exports, |default| {
            llrt_http::fetch::init(ctx, default)?;

            Class::<llrt_http::request::Request>::define(default)?;
            Class::<llrt_http::response::Response>::define(default)?;
            Class::<llrt_http::headers::Headers>::define(default)?;

            llrt_http::blob::init(ctx, default)?;

            Class::<llrt_http::file::File>::define(default)?;

            Ok(())
        })?;

        Ok(())
    }
}

@nabetti1720
Copy link
Contributor

Personally, I believe that the name llrt_http is likely to be mistaken for being compatible with the Node.js http module. Something like llrt_fetch or llrt_libhttp would be preferable. :)

@Sytten
Copy link
Collaborator Author

Sytten commented Oct 29, 2024

I can rename the whole folder I agree

@richarddavison
Copy link
Contributor

@Sytten can you please rebase and rename the directory?

@Sytten
Copy link
Collaborator Author

Sytten commented Dec 30, 2024

@richarddavison I was kinda of waiting to see if we adopted the new module system.

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.

3 participants