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

Should json be default feature ? #47

Closed
pinkforest opened this issue Feb 6, 2023 · 4 comments · Fixed by #49
Closed

Should json be default feature ? #47

pinkforest opened this issue Feb 6, 2023 · 4 comments · Fixed by #49

Comments

@pinkforest
Copy link
Contributor

pinkforest commented Feb 6, 2023

$ cargo update

    Updating crates.io index
      Adding itoa v1.0.5
    Updating markdown v1.0.0-alpha.5 -> v1.0.0-alpha.6
      Adding ryu v1.0.12
      Adding serde v1.0.152
      Adding serde_json v1.0.92

I am using mdast just to parse it and then process the mdast structures, e.g. I don't need json or any serialization out.

Would there be openness to having it as optional considering there are different ways to serialize out ?

Thanks for the great work btw ! 💜

Most of the ecosystem has serde as optional feature

@wooorm
Copy link
Owner

wooorm commented Feb 6, 2023

It is optional?

serde = { version = "1.0", features = ["derive"], optional = true }

@pinkforest
Copy link
Contributor Author

pinkforest commented Feb 6, 2023

Yes but it's on-by-default activated as it's coming via default = ["json"]

@wooorm
Copy link
Owner

wooorm commented Feb 6, 2023

Ah okay. So you can already turn it off, and then get what you are asking above, right?

But you think it should default to not having serde? Why do you want that?

@pinkforest
Copy link
Contributor Author

People are used in the Rust ecosystem way to turn serde on or any other supplementary functionality.

Default feature set should not generally have any supplemental functionality enabled -

serde increases code sizes where I would expect a parser crate only to enable what is required for parsing

Ecosystem normal way is to not have default features that are not needed for the crate's primary function which in this case is parsing and not re-serializing which is optional additive feature especially since people can either choose html or json serialization optionally.

By turning it off we have to specify default-features = false and list all the features explicitly leading to pain to track all the changing features across the relases increasing maintenance overhead.

Code sizes are important for core parsing libraries especially in wasm/embedded contexts.

@wooorm wooorm closed this as completed in #49 Feb 6, 2023
wooorm pushed a commit that referenced this issue Feb 6, 2023
Closes GH-47.
Closes GH-49.

Reviewed-by: Titus Wormer <tituswormer@gmail.com>
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 a pull request may close this issue.

2 participants