deep-c / themes-sass-compiler-brunch

2 stars 1 forks source link

Structure simplication may solve the issue #1

Open MyklClason opened 8 years ago

MyklClason commented 8 years ago

I saw this on stack overflow and decided to answer it over here.

Note: There are better ways to do what this tries to do, at least in Sass. Specifically, I'm referring to @import in Sass (see import section for example). That being said, it seems like there might be a use for this elsewhere. Therefore, you should probably change the name as if implemented well, what you are trying to do shouldn't matter what type of files they are. Also, not sure that "themes" is the right name for what this does.

Proposal

Seems like the solution is to improve the structure. Something like:

app
    /themes
        /base
        /theme2
        /theme3
        /theme-name...

This removes the limitations on what a theme can be named. In this case, anything can be used as a theme name except "base". Although, an option should be used to set the name of the base theme to change that limitation to "any theme can be used except for the predefined base theme". So the brunch config (coffee) might look like:

plugins:
  themes:
    base: "template"

Which would change the base folder to app/themes/template. You could even go a step farther and allow for defining the folder that the base folder is within. However, this may cause conflicts or other problems, but it might look like:

plugins:
  themes:
    themes: "css"
    base: "template"

Which changes the base folder to app/css/template. It could also use something like themes:"a/a/a" and get app/a/a/a/template. It could also be setup so that the base folder is app/base by using themes: "" and base:"base".

Alternatively, you could just merge them into something like base: "css\base". However, it seems better to separate the themes folder from the base folder.

Implementation

This should simply the implementation to simply:

The configuration part could be handled by (not sure if it should be \ or /):

if themes
  dir= "\app\" + themes + base
else
  dir = "\app\" + base

Alternatively. explicitly check if themes is the empty string

if themes == ""
  dir = "\app\" + base
else:
  dir= "\app\" + themes + base
deep-c commented 8 years ago

Hi,

Good points, I definitely agree with your suggestions. I'll work on implementing it and testing it out. With the proposal to recursively copy the files to each theme dir, do you think this would be a slow process and cause the compile time to go up a whole lot? Im going to give it a go and find out but that could be the only issue I see off the top of my head.

MyklClason commented 8 years ago

This seems quite acceptable and efficient when it comes to simply using brunch build. However, it may not be practical to implement as stated, now that I'm looking over the API some more.

For Brunch, the following cases needs to be considered:

Note looking at the Brunch Plugins API...

Seems like the most useful bits are:

The main potential issue I see is how to handle the actual copying so that brunch can continue building using the temp copy.

deep-c commented 8 years ago

Also, for the specific use case I need to build this for I cant have the copied files stay indefinitely, after a brunch build/watch(this is more tricky to deal with) the files that aren't being overridden by a theme other than base need to be removed. Furthermore by doing this the import comments inside the generated css files will be incorrect as they will reference files that arent supposed to exist in a themes directory. The more I think about this the more I am inclined to switch to gulp for this particular use case.since brunch configuration and pipeline flexibility just doesnt cut it for this use case.

MyklClason commented 8 years ago

You might be able to create the copy at the start and delete it at the end using onCompile. Although, this again seems like it would work best with brunch build.

In the end, Gulp may indeed work better for this case. That being said, now I'm curious on how you would do this in Brunch, so I think I'm going to try my hand at it if you have no objections. It'll be good practice for making Brunch plugins I think.

deep-c commented 8 years ago

Even if i did remove the files on onCompile, if i use brunch watch it will end up recompiling because the files have been removed. I started work on this idea in a branch called issu1

deep-c commented 8 years ago

Check out the sass-import branch for a working version that utilises the pipeline while still allowing us to modify the imports.

MyklClason commented 8 years ago

Comments are helpful. Could you add them? Though I think I'm mostly following what your doing, comments would help with clarity as to what you are doing and why.

It's too bad that brunch watch --production seems valid. Otherwise, it seemed like the copied files could be left while in development mode and removed in production mode (via onCompile). Since, it's easy enough to setup separate output folders for development and production. Then again, how often is it that watch is used in production mode? Although, that also figures that the files only really need to be removed in production and it's fine if they stay during development.