[WIP Do not merge yet] Build bundle using rollup #55
Conversation
|
Hi @faergeek, Thanks for your interest in contributing! Before investigating this more thoroughly, I'd like to better understand the problems you're trying to solve. Do you have a specific scenario you're running into where the current build is a problem? As to the specific questions, I can answer those:
In conclusion, I think some changes here are feasible, but I'd prefer them to be as small as possible, and focus on solving a specific problem (such as allowing clients to require the engine without the default rules), rather than trying to match more modern code/module style, which isn't a project goal. So I'd like to hear what problems you're running into trying to use simple-markdown, and see how we could best solve those together. (Or if you're not running into a specific problem but looking to contribute generally, I'd be happy to talk about some less-radical changes that the project could use help with!) |
|
The main problem with the build is that there's no build system right now. With rollup I plan to use es modules syntax, which will be compiled to UMD (for node and browser, including AMD) and es modules (in fact almost nothing will change from source in that case). ES modules allow tree shaking, so that's mostly just about bundle size. On prettier and flow comments. I'm not sure yet about extracting types for external consumption, but there is a plugin for rollup which can strip flow types, so we don't have to use comments. Build step is needed anyway. So I think it solves problems with prettier, project-wide checking and probably external checking (if rollup plugin does not extract types, then we need a separate tool may be). About single file. It will be distributed as a single file anyway. But which single file depends on module support :-) There are two formats (see first paragraph) which will be enough for at least all cases I know. And when talking about separating into multiple files I mean only source, distributed files are single bundles, just different formats for different purposes. If less radical changes mean less to review, I'm not sure it will be possible if I need to get rid of flow comments, replacing them with the usual syntax. So may be separating into multiple files here is a good idea, may be not. I think I will do it as a last step anyway and everything will be in separate commits, so that we can discuss what to accept and what not. |
|
Hi @faergeek, Thanks for the response. I think you may have misunderstood me—I know that the output would be a single file, but I'm concerned that that single file might be less readable or useable than the current one, and before committing to a specific build system I'd need to investigate its effects on the various environments I expect simple-markdown to run in, and whether the default single file build would get larger due to module headers or such. I know rollup is good and reasonably common, but I haven't investigated its output and I'd want to be confident of the results here before committing. Second, it adds a dependency. This means that if someone needs to make a small tweak, they must run the build system themselves, and that if the landscape changes in the future—rollup becomes less popular, or has major changes, or future builds become incompatible with files built with prior versions—then there is a maintenance burden in upgrading. I think these prices are potentially small, but they add complexity, and I don't want to add that complexity unless there's a compelling reason. If users of the library really need smaller bundles, then I'd like to consider how to deliver that to them, whether via tree-shaking or some other build system, depending on what exactly they need and how best to address it. And I would like to hear from users having trouble with this directly to fully understand their issues first. Until that's the case, though, committing to something early locks us into whatever that decision is, and adds a maintenance burden that may not have an immediate benefit. (As an aside, there's a variety of reasons why tree-shaking doesn't seem to me to have a lot of potential benefit on this particular project. From my understanding, most users of simple-markdown currently use almost all of the code, and therefore wouldn't gain much benefit from tree-shaking. I do know of one large user that doesn't use many of the default rules, but I don't know what their build system is or how it works, so I don't know that building tree-shaking would necessarily benefit them. In addition, much of the code that is only conditionally used is nested inside objects, which would be challenging to pull out into modules themselves. For these reasons, I'm not sure that tree-shaking support would be the best way to provide smaller bundles to users, and I'd want to know exactly what problems they're having, how much savings they need, and what their build system is, before committing to a solution. simple-markdown is a relatively tiny markdown parser already – 5 KB compared to marked.js's ~7 KB, or markdown-it's ~25 KB) In summary: I'd like to wait for specific user needs before making these types of changes, because they do have a cost, and because I want to make sure to solve their actual problems as best as possible. I can see you're excited about this, and it sounds like a good project to learn things! You're welcome to try it, but I don't think I'm likely to merge a build system pull request into the core repo until it has a clear benefit for users of the library. If you're interested in other contributions, there are some tests and bugfixes that need work which I could help with! |
|
This doesn't lock project to rollup. There are at least webpack, browserify (with babel), which can understand code equally well, but rollup is famous for inventing tree-shaking and really good at that, plus it's better for libraries, while webpack is better for apps. After running rollup I don't see anything unreadable, may be a few one liners at the top, but they are still readable, not even minified. So, even getting rid of build system in the future and editing bundle is possible. You'll lose only flow types in such case. And there's also almost no overhead. Just a few lines and IIFE. And benefit is not in build system, actually benefit is in plain standard es modules, which I'd say will be almost equal to source. Build system is rather for backwards compatibility (UMD etc.). Webpack works with such bundles out of the box (I mean tree-shaking) and it's most widespread bundler. Plus architecture of this project allows to benefit from this from the start. Here we have many exports which user may need or may not need (they will not be included in app). At this stage there's even nothing needed to change in library interface to benefit from tree-shaking. A little bit later I can build an example to prove all of that if you agree with some parts of what I written here :-) About other contributions, what about migrating to jest? It's just plain simpler and has a good watch mode, plus code coverage out of the box, etc. Oh, and I would suggest to get rid of Makefile and just use npm scripts. It's good system, but not for JavaScript, better for C or other compiled languages with intermediary binaries :-) |
|
As I said before, I'm not interested to migrating to a more complex build system until I know what troubles users are running into with the current build, so I can make sure to solve those well.
I don't believe that's true. Of the 28 exports, 3 are core to the rules/engine (and could potentially benefit from tree shaking, if users are using only 1-2 of them, which is uncommon), 11 are helper functions which are called internally anyways and couldn't be removed by tree-shaking, 10 are convenience wrappers of less than 5 lines, and the remaining 4 functions are <30 lines, three of which are deprecated and will be removed in 1.0. So no, the current architecture of this project does not benefit significantly from tree-shaking. You could potentially get tree-shaking benefits under specific circumstances by breaking up the defaultRules object, but that changes the API significantly, and I'd like to wait until I know what people need before doing that.
Jest already works;
This sounds extremely condescending. I don't need you to explain technical concepts that I understand fine to me on my own project's page. It's rude to come into a project and tell them to overhaul the project's infrastructure. Why should I have to switch all the systems I've used to maintain this project for years because someone tells me that they think it would be better if I used their favourite tools? You've been entitled and condescending here, and it's hurt. |
Sorry, I didn't want to enforce some tool or condescend. English is not my native language, so what I write may sound like shit probably. My main concern about |
It's the solution for #14 which I proposed. It's far from being ready.
I'd like some guidelines on what to do and what not to do: