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

Update CI test for wasm32-unknown-unknown to enable wasmbind feature #741

Closed
wants to merge 1 commit into from

Conversation

AmateurECE
Copy link
Contributor

This PR is to fix #740. All that's been committed so far is a change to the CI configuration that will allow discovering compilation errors like this again in the future. Once a CI run has been scheduled and we have observed the failure in the logs, I will commit the fixing change.

  • Have you added yourself and the change to the changelog? (Don't worry
    about adding the PR number)
  • If this pull request fixes a bug, does it add a test that verifies that
    we can't reintroduce it?

Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

I guess this seems reasonable. Do we always require --features wasmbind now for wasm32-unknown-unknown? If so, should we unconditionally enable those dependencies for that target instead to make this easier to work with?

@AmateurECE
Copy link
Contributor Author

Great question! I was thinking about that, too. Seems those features enable functions for working with types like js_sys::Date from the rustwasm family of crates. You can use chrono without enabling this feature flag in a wasm32-unknown-unknown environment, but I ended up getting surprise unimplemented! panics for attempting to call functions of chrono's public interface that depended on this feature under the hood.

That being said, I would support unconditionally enabling this feature for that target, as you suggested.

@djc
Copy link
Contributor

djc commented Jul 26, 2022

That being said, I would support unconditionally enabling this feature for that target, as you suggested.

Okay, let's do it!

@djc
Copy link
Contributor

djc commented Jul 28, 2022

@esheppa worked on #746, so I think this can be closed. Can you check if current main works for your use case?

@AmateurECE
Copy link
Contributor Author

Looks like #746 fixes my issue, thank you very much for checking in! It's safe to close this PR, then.

@esheppa esheppa closed this Jul 28, 2022
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.

wasm32/wasmbind: cannot find function tm_to_datetime in this scope
3 participants