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 #120 - don't compile in JsValue in proc_macro. #121

Closed

Conversation

finnbear
Copy link
Contributor

Fixes #120

If merged, will ultimately require cargo publish of new versions for all three crates.

@finnbear
Copy link
Contributor Author

finnbear commented May 24, 2023

I incorrectly assumed that enabling a feature in the proc macro wouldn't also enable it in the main (stylist) crate. Will use a slightly different approach...

... done!

@finnbear finnbear marked this pull request as ready for review May 24, 2023 05:43
@futursolo
Copy link
Owner

futursolo commented May 24, 2023

I think this issue has got attention from the compiler team.
I recommend we see if they will prioritise as I do not think this particular regression should make into a release.

If they cannot fix it in a couple days then we can merge this mitigation.

In the meanwhile, would it be possible for you use a nightly version prior to this issue occurring?

@finnbear
Copy link
Contributor Author

finnbear commented May 24, 2023

In the meanwhile, would it be possible for you use a nightly version prior to this issue occurring?

Don't worry about me! While I can't change nightly versions, I have pointed Cargo.toml at my github branch and am now up and running.

Copy link
Owner

@futursolo futursolo left a comment

Choose a reason for hiding this comment

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

I incorrectly assumed that enabling a feature in the proc macro wouldn't also enable it in the main (stylist) crate. Will use a slightly different approach...

... done!

With Rust 1.70, this regression has made into stable. So I think we should mitigate this.

Between the feature approach and target approach, I prefer the feature approach as this would allow others to write code with Error::Web without having to write #[cfg(...)] and this will disable language server / clippy for native targets.

I was actually able to build individual examples but not the workspace somehow.
Although cargo build for the entire workspace should also work as stylist uses cargo resolver 2.

Maybe we can have a mix of both approaches by applying the #[cfg(feature)] to type JsValue?

@finnbear
Copy link
Contributor Author

finnbear commented Jun 2, 2023

I prefer the feature approach

I initially implemented a feature-based approach but it didn't work for reasons previously described - the feature appeared to be enabled when compiling stylist, not just when compiling the proc-macro crate.

It sounds like you may have a solution in mind, but I don't fully understand it, so feel free to fix this issue in whatever way you like in a different PR!

universal-git added a commit to universal-git/stylist-rs that referenced this pull request Jun 5, 2023
…#121

This gets rid of the compile error:
/usr/bin/ld: __wbindgen_realloc: undefined version: 
          /usr/bin/ld: __wbindgen_malloc: undefined version: 
          /usr/bin/ld: __wbindgen_free: undefined version: 
          /usr/bin/ld: __wbindgen_exn_store: undefined version: 
          /usr/bin/ld: failed to set dynamic section sizes: bad value
          collect2: error: ld returned 1 exit status
          
error: could not compile `stylist-macros` (lib) due to previous error

This is just a test-verification of the below commit by @finnbear

futursolo@9272103
Copy link

@universal-git universal-git left a comment

Choose a reason for hiding this comment

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

This works, gets rid of the compile-error:
"/usr/bin/ld: __wbindgen_realloc: undefined version:
/usr/bin/ld: __wbindgen_malloc: undefined version:
/usr/bin/ld: __wbindgen_free: undefined version:
/usr/bin/ld: __wbindgen_exn_store: undefined version:
/usr/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status
error: could not compile stylist-macros (lib) due to previous error"

Change is only in stylist-core/src/error.rs.
universal-git@a22aba4

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.

Importing JsValue in a proc_macro causes a compilation error on recent nightly's
3 participants