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

add wasmbind feature #4875

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

add wasmbind feature #4875

wants to merge 5 commits into from

Conversation

bdemann
Copy link

@bdemann bdemann commented Apr 14, 2023

Resolves #4320

}
// Date.now returns unix time in milliseconds, we want it in seconds
Ok(Date::now() / 1000.0)
panic!("Date not available")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why panic?

Copy link
Author

Choose a reason for hiding this comment

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

If you are compiling wasm32-unknown-unknown and not wasi then std time doesn't work, so there is no way to get a date generally as far as I know for for wasm32-unknown-unknown.

Copy link

@lastmjs lastmjs Apr 14, 2023

Choose a reason for hiding this comment

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

Adding to @bdemann's reasoning (we're on the same team), in our project, which must compile and run in a wasm32-unknown-unknown environment, std time doesn't work, and we have our own time implementation that we import from the host environment. We would love for a solution to this problem as well, but this PR is scoped for general use.

Copy link
Contributor

Choose a reason for hiding this comment

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

When the interpreter panics, the interpreter process comes to a complete emergency stop. From there, you have to start it again. Is this what you want?

Copy link
Member

Choose a reason for hiding this comment

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

Typically, unsupported platforms in Python just don't expose the module/function/constant etc (the os module is an example of this). If the time module doesn't work for the target it would make more sense to just not have it be available.

Baring that, an exception would be the second best option.

Copy link
Member

Choose a reason for hiding this comment

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

I also vote for hiding the function time.time when not available.
Then user can set it like time.time = custom_time when not hasattr(time, 'time')

Copy link
Member

@youknowone youknowone Apr 16, 2023

Choose a reason for hiding this comment

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

Ah, I found process_time and process_time_ns raises error. NotImplementedError also will be ok.

}
let mut s = String::new();
self.write_exception(&mut s, &exc).unwrap();
error(&s);
panic!("{}; exception backtrace above", msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail to compile if the wasmbind feature is not enabled.

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.

Support deployment to wasm32-unknown-unknown without wasm-bindgen
5 participants