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

Break out the core book from everything else #1234

Open
Michael-F-Bryan opened this issue May 21, 2020 · 3 comments
Open

Break out the core book from everything else #1234

Michael-F-Bryan opened this issue May 21, 2020 · 3 comments
Labels
Breaking Change This would require a SemVer breaking change C-refactor Category: Code refactoring
Milestone

Comments

@Michael-F-Bryan
Copy link
Contributor

In the same vein as #1211 and #1076, a massive pain point when developing plugins is that you need to import mdbook (which is fine) and this pulls in half of crates.io as transitive dependencies (which is not).

My thoughts are we could have a mdbook-core crate containing the bare minimum needed to load a book from disk, plus common traits:

  • the Book type and things it contains (BookItem, Chapter, etc.)
  • the SUMMARY.md parser and related code for loading a book from disk
  • the Renderer trait
  • the Preprocessor trait
  • the CmdRenderer - because it only requires std and we'll already be pulling in serde

This then opens the way for making the HTML renderer just another renderer (e.g. by installing a mdbook-html binary alongside mdbook).

It's quite a big change in terms of work and backwards compatibility (although we could probably re-export items from mdbook-core in mdbook?) and I'm happy with doing most of the grunt work, but thought I'd get some thoughts before making changes.

Does this seem like a reasonable idea? Or would it just be a bunch of busy work which is going to break a lot of in-flight PRs and waste time?

@ehuss
Copy link
Contributor

ehuss commented May 21, 2020

I think it would be great to have a more stripped-down core library. I'm uncertain if it is necessary to make it a separate crate. My concern is that adds some maintenance overhead. Would it be OK to just use features more aggressively? I assume the main dependencies to avoid are:

  • clap (I didn't know this was always built!)
  • handlebars
  • env_logger?
  • pulldown-cmark? Is this possible?
  • regex
  • tempfile, shlex, other small ones

That seems relatively easy to make those optional, default features.

@Michael-F-Bryan
Copy link
Contributor Author

I think the biggest offenders are all the extras used by the binary, and the HTML renderer. For example, a library doesn't actually need to instantiate a logger (env_logger), parse arguments (clap), or spin up a web server.

From what I can tell the only thing you really need for the Book and configuration are pulldown-cmark (which is really cheap because it's just text processing) and serde (expensive, but configuration is coupled to it). Likewise, the Renderer and Preprocessor traits a result just dumb objects, possibly with a #[derive(Serialize, Deserialize)] or two.

I'm uncertain if it is necessary to make it a separate crate. My concern is that adds some maintenance overhead

I'm not sold on this... We use workspaces a lot to keep compile times down and maintain separation in work projects and in practice it's no different from putting things into modules, they're just different folders on disc.

Tools like cargo-release also make releasing a dozen crates in a workspace is as easy as releasing one crate.

Would it be OK to just use features more aggressively?

I've found using more than one or two feature flags in a crate tends to make things harder to maintain. It clutters the code a lot, and you've got to be careful to try different permutations of feature flags in case one feature accidentally uses something that only exists with another feature.

Often you'll just run cargo test --no-default-features and cargo test --all-features in CI, so these oddball permutations are never checked.


This weekend I'll do an experiment to see what the bare minimum in terms of functionality and dependencies is needed for using mdbook as a library (r.g. for plugins). We should be able to take it from there.

@ehuss ehuss added Breaking Change This would require a SemVer breaking change C-refactor Category: Code refactoring labels Jul 27, 2021
@willcrichton
Copy link
Contributor

I did a proof-of-concept in #1937 to see how many dependencies I could strip away while enabling my mdbook-quiz preprocessor to still work. I removed:

  • chrono
  • clap / clap_complete
  • env_logger
  • handlebars
  • opener
  • tempfile
  • topological-sort

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change This would require a SemVer breaking change C-refactor Category: Code refactoring
Projects
None yet
Development

No branches or pull requests

3 participants