blakeembrey / free-style

Make CSS easier and more maintainable by using JavaScript
MIT License
707 stars 29 forks source link

Componentize into files (wip as of now.../reviews & opinions plz) #51

Closed devdoomari closed 7 years ago

devdoomari commented 7 years ago

I'm working on componentizing FreeStyle intro multiple files... (1 class - 1 file)

While I'm working, please give some suggestions/ opinions on how it should be done/not done, and other corrections : )

blakeembrey commented 7 years ago

Can you explain why I'd want this? The original file is much easier to maintain and manage than having half a dozen files. Keeping it under 500 lines makes it a nice goal too.

Side note: When defining modules, please use dash case instead of pascal case for file names.

devdoomari commented 7 years ago

Here's the reasoning from SO: http://stackoverflow.com/questions/360643/is-it-a-bad-practice-to-have-multiple-classes-in-the-same-file

my personal reason:

Also, I think it's better to keep files within <150 lines for poor ppl like me who don't have a wide-screen tiltable monitor :(

blakeembrey commented 7 years ago

I don't think that's really required, and feels like overkill in a language like JavaScript and a project like this. This project is simple and I like that it can be demonstrated and read like a short novel. On the points:

FWIW, I didn't write this on a large monitor - most of my work I do on a regular 13 inch Macbook. In it's current form, I just don't see a benefit myself. If this were a larger or more complex project, I would certainly agree, but I don't feel that 500 lines is too large here.

Edit: I am, of course, welcome to hear other balanced opinions.

devdoomari commented 7 years ago

ok...so here was my plan:

I was thinking about adding 'ordering' for media queries. (I'm using TypeStyle, but that library is almost FreeStyle-under-the-hood)

Here's more explanation of my situation: https://github.com/typestyle/typestyle/issues/129

Anyway, I think this might be quite a large change in both libraries ( a lot of additions - especially using arrays-of-style-objects instead of dictionaries to preserve order).

I don't think I can keep this library 500< lines after the change, and I wanted to make this library more easier to edit before working on.

blakeembrey commented 7 years ago

I'm not really sure what you're after, but the order is not random nor is it based on a dictionary. There's a flag set up to keep order with nested keys and the cache is always ordered by last insertion. How about you submit an issue with the feature or issue you're facing and we can track that down first?

devdoomari commented 7 years ago

ok... so I guess I was working on the wrong one...

but what API should I use? registerStyle({ ... }) is dictionary-only...

is it possible to 'add' more to a generated classname?

e.g)

const someClass1Name = registerStyle({ ... });
...
freeStyle.addMoreTo( someClass1Name, {
    '@media for iphone5 ', { ... }
);
freeStyle.addMoreTo( someClass1Name, {
    '@media for ipad ', { ... }
);

EDIT: link to issue: https://github.com/blakeembrey/free-style/issues/52

blakeembrey commented 7 years ago

I see. Yes, the input itself is dictionary only, but all JavaScript engines tend to sort by definition order. I can look at adding a more explicit API if there's a proposal for what that would look like.