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

fix: prevent compiling wasm only code on unsupported platforms #225

Closed
wants to merge 2 commits into from

Conversation

rtyler
Copy link

@rtyler rtyler commented Mar 11, 2025

@rtyler rtyler requested a review from a team as a code owner March 11, 2025 22:01
@ddimaria
Copy link

Publish dry run is failing, maybe all features are turned on? Reqwest removed this function completely: seanmonstar/reqwest@0bcba46

@@ -527,6 +527,7 @@ impl RequestBuilder {
/// The [request mode][mdn] will be set to 'no-cors'.
///
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/API/Request/mode
#[cfg(target_family = "wasm")]

Choose a reason for hiding this comment

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

reqwest uses this cfg attribute https://github.com/seanmonstar/reqwest/blob/master/src/lib.rs#L255C9-L255C39:

#[cfg(target_arch = "wasm32")]

I wonder if the issue is that these don't match

@rtyler rtyler force-pushed the conditional-wasm branch from 2219899 to a7597a7 Compare March 12, 2025 17:55
@douggynix
Copy link

douggynix commented Mar 14, 2025

@benbrandt should this PR be reviewed and obtained an approval from you for it to get merged?
I am waiting as well for this pending fix to fix my build failures. would you review/approve it please?

@benbrandt
Copy link

@douggynix I doubt it because I'm not a maintainer but I did anyway

@douggynix
Copy link

Can we include the maintainers in the comments to have this PR reviewed. I am having build errors which this PR has fixed. A review to update the library crate would be very useful for me and others experiencing build issues with the latest versions of reqwest.
Thanks

@douggynix
Copy link

@eopb : would you have a look at this PR for approbation or change request please?

@douggynix
Copy link

@rtyler : you got pending requested changes to fix the CI issue. any update on this please?

@douggynix
Copy link

why this PR is still pending for so long knowing that there are build failures issues for the new versions of reqwest?

@jdon
Copy link
Member

jdon commented Apr 8, 2025

Closing as this has now been fixed in reqwest-middleware 0.4.2

I didn't have permission to push changes to this branch, so I've fixed this in a separate PR: #230.

@jdon jdon closed this Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

reqwest 0.12.13 breaks compatibility (fetch_mode_no_cors removed)
5 participants