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 unexpected_cfgs in the crates and add a troubleshooting note #783

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

tguichaoua
Copy link
Contributor

  • Moves the unexpected_cfgs lint configration at the workspace level
  • Inherits the workspace lints in all crates
  • Add a notes about the "unexpected cfg condition name: sycamore_force_ssr" in the troubleshooting section of the docs.

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.41%. Comparing base (b0aa222) to head (fad8af9).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #783   +/-   ##
=======================================
  Coverage   71.41%   71.41%           
=======================================
  Files          45       45           
  Lines        6658     6658           
=======================================
  Hits         4755     4755           
  Misses       1903     1903           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lukechu10
Copy link
Member

Thanks for the PR and sorry for taking so long to get to this. I'm happy to merge this for now but I don't quite like this solution long term as it introduces extra friction to get started with Sycamore.

It seems like the main problem is that certain macros expand to cfg(sycamore_force_ssr) which then triggers the warning. Think behavior was introduced in rust-lang/rust#132572

I think the best solution would be to not expand to cfg(sycamore_force_ssr) but rather to true or false depending on the flag. This way we do not get warnings in downstream crates.

@lukechu10 lukechu10 merged commit 116ecd3 into sycamore-rs:main Jan 28, 2025
8 of 9 checks passed
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.

2 participants