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

Use a nightly feature flag instead of stable #567

Closed
remkop22 opened this issue Feb 22, 2023 · 5 comments · Fixed by #1227
Closed

Use a nightly feature flag instead of stable #567

remkop22 opened this issue Feb 22, 2023 · 5 comments · Fixed by #1227
Milestone

Comments

@remkop22
Copy link
Contributor

remkop22 commented Feb 22, 2023

We had some good discussion on the leptos discord about the current stable feature flag. I'll try to reproduce the conversation here.

Problem

Let's say you have this dependency graph:

my-crate
 |
 |--- foo
 |     |---- leptos (features = ["stable"])
 |
 |--- bar
       |---- leptos

Because features are additive and foo used stable, the whole build tree now builds as stable. If bar used nighly features in it's code, now the build fails.

The key is in this line of the cargo book:

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features

Currently this is not the case since enabling the stable feature flag actually disables functionality.

Possible Solution

Remove stable and introduce a feature flag called nightly that is on by default and switch all conditional compilation from #[cfg(not(feature = "stable"))] to #[cfg(feature = "nightly")]

This is a breaking change since everybody that either uses stable or has disabled default features has to change their Cargo.toml in order to keep using nightly features.

@gbj
Copy link
Collaborator

gbj commented Feb 24, 2023

So my experience from doing a quick implementation of this is:

  1. This is probably "correct" from the "cargo best practices" point of view
  2. Enabling nightly as a default feature makes it slightly more ergonomic, especially if we remove csr from default features, which allows you to use ssr and hydrate modes without needing to specify default-features = false (however it means that for simple browser-only examples you need to enable csr).
  3. However, this also breaks almost every single doctest in our current CI, which uses cargo-all-features. Currently the stable feature is just disabled for CI, so all the tests aren't run with stable. However, you can't disable cargo-all-features from testing the empty features set []. This means that all the doctests will be run with the default (stable only) I'm not quite adept enough to address this.

@remkop22
Copy link
Contributor Author

Hmm I guess the correct option is to fully feature gate all doc-test's. But then, as you said before leptos is currently developed with the idea that nightly is the default toolchain. So that's quite a lot of work for not that big of a return...

I've looked at cargo-hack which has these options that I think can be used in conjunction:

--feature-powerset
    Perform for the feature powerset of the package.

    This also includes runs with just --no-default-features flag, and default features.

    When this flag is used together with --depth or namespaced features (-Z
    namespaced-features) and not used together with --exclude-features (--skip) and
    --include-features and there are multiple features, this also includes runs with just
    --all-features flag.

And

--exclude-no-default-features
    Exclude run of just --no-default-features flag.

    This flag can only be used together with either --each-feature flag or
    --feature-powerset flag.

I'll try and see if I can swap this out with cargo-all-features without much hassle.

@remkop22
Copy link
Contributor Author

I'll try and see if I can swap this out with cargo-all-features without much hassle.

Yeah, this didn't work. I eventually made it work with cargo-all-features

@gbj gbj added this to the 0.3.0 milestone Apr 14, 2023
@afiqzx
Copy link
Contributor

afiqzx commented May 19, 2023

Has this been fixed? It is set to 0.3.0 milestone.

@gbj gbj modified the milestones: 0.3.0, 0.4.0 May 19, 2023
@gbj
Copy link
Collaborator

gbj commented May 19, 2023

Has this been fixed? It is set to 0.3.0 milestone.

Copying and pasting from my reply to this question on Discord: "I released 0.3.0 rather than waiting longer and longer to pack in additional breaking changes. I have pretty limited time that’s largely taken up by support, so haven’t gone in and updated that tag."

Updated now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants