-
Notifications
You must be signed in to change notification settings - Fork 75
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
Refactor library modules #194
Comments
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 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.
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.
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?
It’s easy to ignore the code, and putting the website (which includes the docs) somewhere else would just make it harder to find.
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! |
@brainkim, thank you so much for such a thorough response, I greatly appreciate it! There's sill something I can add here (answering to your points):
Some specific responses
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.
Well, they provide a way to easily fix it, so why not, I guess? :) |
Maybe relevant: Elm Europe 2017 - Evan Czaplicki - The life of a file https://www.youtube.com/watch?v=XpDsk374LDE |
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. |
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
, anddom.ts
- and a "barrel" fileindex.ts
, with the bulk of the code resting incrank.ts
(over 2k lines at the time of writing).This creates several maintainability issues:
Build
Current build process has several flaws:
package.json
,.gitignore
and several other files."unpkg"
field inpackage.json
leads tounpkg.com
servingcjs
version by default, instead ofumd
oresm
.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
"esm"
directory be the output for theesm
build and change the corresponding fields inpackage.json
and other files."sideEffects": false
to improve static analysis and tree-shaking."unpkg"
field.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.
The text was updated successfully, but these errors were encountered: