bikeshaving / crank

The Just JavaScript Framework
https://crank.js.org
MIT License
2.7k stars 75 forks source link

Refactor library modules #194

Closed Raiondesu closed 10 months ago

Raiondesu commented 3 years ago

Preamble

I am aware that this particular topic is nothing groundbreaking and that crank is still in its early development stages. 🙂\ But this issue is going to get a higher priority as Crank gets more attention from people, so I feel like this has to be written down somewhere, for tracking purposes at least.

Current Problems

Code grouping

Crank currently has 3 main modules - crank.ts, html.ts, and dom.ts - and a "barrel" file index.ts, with the bulk of the code resting in crank.ts (over 2k lines at the time of writing).

This creates several maintainability issues:

  1. Difficulties for new potential contributors.
  2. Unclear separation of concerns.
  3. Code scalability will become harder to maintain as amount of contributors increases.
  4. Ecosystem is difficult to build and sustain without a proper modular API.

Build

Current build process has several flaws:

  1. ESM build is spit into the root project directory, which results in any change to current module structure having to be "relayed" to package.json, .gitignore and several other files.
  2. Lack of the "unpkg" field in package.json leads to unpkg.com serving cjs version by default, instead of umd or esm.

    Website and examples

are embedded into the main repository, which simply results in unnecessary code for anybody willing to clone Crank and contribute to it.

Possible solutions

  1. Code grouping: just divide the code into smaller modules, split by their responsibility scope.
  2. Build:
    • make "esm" directory be the output for the esm build and change the corresponding fields in package.json and other files.
    • Add "sideEffects": false to improve static analysis and tree-shaking.
    • Add the "unpkg" field.
  3. Website and examples: split them into separate repositories, might be useful to take another look at #81.

P.S. I suspect that there's a purpose/reasoning behind the current project structure, even though it doesn't seem to be stated anywhere. Even if it is true, however, problems described above still stand.

brainkim commented 3 years ago

I appreciate the enthusiasm and thoughtfulness of this issue. And I unironically enjoyed pulling down your pull request and looking at the Crank.js repository spread out into all those files. However, I disagree with the idea that the core file being ~2k lines of code (most of which are comments) makes Crank difficult to read, contribute to, or build an ecosystem around.

This is largely a matter of opinion of course, and I think one of the things people leave out when they discuss their preferences for few large files or many small files is their personal practice, how they interface with code. I can understand why some people might find a lot of code in a single file to be difficult to read. Sometimes, you struggle to figure out what function or class you’re in when there are multiple abstractions in a single file, as opposed to reading the filename and getting a sense of where you are. And of course, you saw the /*** ***/ comment markers which seemed to indicate natural places to split up the file.

Nevertheless, I think it makes the most sense for the core module to be a single file. For the practical concern of getting a bigger picture of large files, I recommend code editing tools like code folding and split editing. This will likely not be your last time encountering a “large” file of code, and these techniques are indispensable for getting a sense of large files at a glance. And personally, I am a voracious reader of code, and I often browser repositories directly on GitHub to learn a particular algorithm or technique. When a library has a technique I’m interested in and flaunts a sub 5KB minzipped size but ends up strewn across multiple files, I usually end up sighing in exasperation.

Additionally, there are many reasons specific to Crank which I think makes it better as as single file.

  1. While there are concrete runtime abstractions like elements, renderers, and contexts which could each be in their own file, these abstractions are mutually recursive and difficult to use independently. If you’re using elements, you’re likely using renderers and contexts. If you’re using renderers, you’re definitely using elements and likely using contexts. And if you’re using contexts, you’re certainly using both elements and renderers. In other words, if you use any part of Crank, you’re likely using all of it, so none of the abstractions are meaningful on their own. You may have noticed this when splitting up the code into various modules, insofar each of the modules circularly depends on at least the types of other modules.

  2. Modules in JavaScript are not just a means of code organization, but delivery. ESModules are now being adopted, and what this means is that it’s possible, for instance, to import Crank directly in the browser thanks to unpkg or other NPM-based CDNs. What this means is that splitting up the core Crank file into multiple files can mean multiple requests. To what end? You can of course, bundle the files to reduce the number of public modules, but again, I have to ask, what is the purpose of splitting up these files in the first place if you’re just going to bring them back together with the build process.

  3. It might surprise you, but I actually take pride in Crank.js’s core being a single file. I have rewritten this file about 3 or 4 times as I learned to write a web framework, and its current iteration is the result of radical and deliberate deletions and refactorings. My goal for Crank was always to write a framework which was so simple that I could rewrite it from scratch if I had to, like a soldier field stripping and reassembling a rifle, and I think I have achieved this to a certain extent. Moreover, having libraries be a single file continues in a venerable tradition, from the days of Backbone, when you could read a file and be given an explanation of its logic. A file can tell a story, insofar as it is linear, in a way which a directory of files does not. This idea is called literate programming, and it’s an idea introduced back in 1984 believe it or not. While I’m not sure yet that Crank fully meets these aspirations in having the source be self-explanatory, I do think that having the core be a single file draws us closer to these ideals.

  4. Another potentially surprising detail, I consider Crank.js to be close to feature complete. The API is much more stable than you might expect. I have always strived to write code which is easy to maintain, and doesn’t require elaborate committees and RFC processes to maintain. I am skeptical of libraries which seem to continuously add features and cause major breaking redesigns. At some point, you have to ask, do these people actually know what they want? Are they happy to continue to chase trends and mind share, at the cost of the people who’ve already bought in? There are some details about coordination of async coordination and unmounting, and error handling which I’d like to iron out at some point, but I’d like for Crank to be feature complete by the end of 2022.

Some specific responses.

ESM build is spit into the root project directory, which results in any change to current module structure having to be "relayed" to package.json, .gitignore and several other files.

Yeah, I’m unhappy about this. If you have specific solutions that don’t involve splitting up Crank into a bunch of files, I’m happy to take a look. I do like the transparency of having files be exactly the same path under the root though, so I dunno.

Lack of the "unpkg" field in package.json leads to unpkg.com serving cjs version by default, instead of umd or esm.

This feels like an unpkg problem, no? I’ve done all the standard things to indicate the package is ESM first, including "type": "module", why isn’t unpkg able to pick that up?

Website and examples are embedded into the main repository, which simply results in unnecessary code for anybody willing to clone Crank and contribute to it.

It’s easy to ignore the code, and putting the website (which includes the docs) somewhere else would just make it harder to find.

"sideEffects": false

We can look into this. Like I said, if you’re using any of Crank, you’re likely to use all of it (excluding specific renderers), so tree-shaking doesn’t really make sense.


Again, thanks so much for bringing this up. I’m sure other people are thinking similar things and you encouraged me to articulate the reasons surrounding the decisions. However, the majority of the changes you propose is almost like asking me to cut my own baby in half, so it’s unlikely to ever be done. Sorry!

Raiondesu commented 3 years ago

@brainkim, thank you so much for such a thorough response, I greatly appreciate it!\ Now I (and, I'm sure, many others will) understand the reasoning behind certain decisions thanks to your response.

There's sill something I can add here (answering to your points):

  1. I actually was expecting more "entanglement" in the source code once I split it up. The only real co-dependence right now is between Context and Renderer classes, which for now truly seem inseparable, as you pointed out. But when it comes to other circular dependencies, they can be undone by simply making each of them depend on abstractions rather than themselves. I'd do it, but I just didn't think it would be appropriate to also right away "mangle your baby" as well as "cut it in half". :)\ I hope this makes sense to you.

  2. If we were to talk about ESModules and direct browser module experience, splitting the code into files allows them to be cached separately, so when an update to one module is published, all of the other modules can still be cached in userland and not require to be downloaded again, in contrast to when the whole source code is in a single file.

    To what end?

    As long as the end justifies the mean, I think. :) This whole "separate caching" thing is essentially one of the reasons why ESModules are a thing in the first place.

  3. The current problem with Crank and literality is that, well, I'd argue its source is not linear. One cannot read the code of Context and completely understand it, unless one reads the code of Renderer, which also cannot be completely understood unless one understands Context. And so is true for some other entities in current implementation. So, as a complete newcomer to Crank, I found myself reading the code in a vertical zig-zag pattern (constantly switching between two entities to understand what's going on). I agree, however, that the code is very compact for what it does and can indeed be rewritten "from scratch if [one] had to, like a soldier field stripping and reassembling a rifle", which can in certain situations be much more preferable than just "comfort of reading it for the first time".

  4. This, in fact, has surprised me. :) However, Crank, at least right now, doesn't seem to be that focused on achieving some specific API either. I guess, my misunderstanding of Crank's completeness is a nice example of this. This has already been brought up in https://github.com/bikeshaving/crank/issues/60#issuecomment-620449233. I'm eager to contribute to Crank and resolve most of the problems we've pointed out, but I lack direction right now, as will many potential contributors. So it will become increasingly necessary to have a list of goals and non-goals somewhere up-front, as the community around Crank gets bigger.


Some specific responses

Yeah, I’m unhappy about this. If you have specific solutions that don’t involve splitting up Crank into a bunch of files, I’m happy to take a look. I do like the transparency of having files be exactly the same path under the root though, so I dunno.

I'd like to keep the #195 open for now, but I'll try to propose an additional one that only fixes the build structure "relaying" problem with as little intervention to the current structure as possible.

This feels like an unpkg problem, no?

Well, they provide a way to easily fix it, so why not, I guess? :)

rofrol commented 2 years ago

Maybe relevant: Elm Europe 2017 - Evan Czaplicki - The life of a file https://www.youtube.com/watch?v=XpDsk374LDE

brainkim commented 10 months ago

Closing for housekeeping purposes. Thanks again for the attempt but I’m still gung-ho on having the core Crank module being a single file.