Skip to content
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

Ultimate Fix Proposal: Vendor CSS issue during build & compile #403

Open
wants to merge 1 commit into
base: v0.3.2-release
Choose a base branch
from

Conversation

TheReluctantHeroes
Copy link

I noticed that there are a bunch discussions about issues relevant to organizing vendor CSS via grunt build & compile tasks but all PRs as far as I can see did not get the point and makes the situation worse.

Thus I raised this PR. Should be able to solve issues mentioned in
#340
#305
67739b8

My PR is implemented based on discussion in #185

As far as I know the correct way of loading external Vendor CSS is that
we define it in build.config.js->vender_files->css (other than
explicitly import it in main.less, etc); however there is something
wrong with loading vendor css via grunt tasks in commit
268901b.

Weird Behavior before this fix:

  • vendor CSS will be concatenated(merged) instead of copied in build
  • index.html still has reference to vendor CSS as standalone files in
    build
  • vendor CSS will disappear if compile without build first because
    'copy:compile_assets' task simply copies concatenated all-in-one css
    file from build folder

Current Behavior after this fix:

  • build no longer concatenates the vendor CSS while it now has correct,
    standalone reference in index.html to vendor CSS copied to its vendor
    folder
  • compile has always vendor CSS concatenated (merged) and integrated
    regardless of whether build task has been performed

The modification has been tested under following cases:

  • watch (delta)
  • default (build & compile)
  • compile after build
  • compare w/o build

Still, please kindly notice:

  • watch on LESS change will not notify to concatenate vendor CSS because
    they are not relevant in build
    From personal point of view I would not support following PRs, which did not suffice to be merged in the main branch:
    re-concat CSS files when LESS files change #369
    add concatenating css on less change #390
  • Regardless of what's mentioned in the documentation, compile task
    still partially depends on build task for javascript files in src/ while
    all vendor files (incl. CSS, JS) can be compiled successfully all the
    time. This is due to historical reasons that 'ngAnnotate' and
    'concat:compile_js' loads file from build folder. As I'm quite unsure if
    there exists modification during the process of copying those relevant
    app js from original src to build, I just leave it remained as for now.

As far as I know the correct way of loading external Vendor CSS is that
we define it in build.config.js->vender_files->css (other than
explicitly import it in main.less, etc); however there is something
wrong with loading vendor css via grunt tasks in commit
268901b.

Weird Behavior before this fix:
- vendor CSS will be concatenated(merged) instead of copied in build
- index.html still has reference to vendor CSS as standalone files in
build
- vendor CSS will disappear if compile without build first because
'copy:compile_assets' task simply copies concatenated all-in-one css
file from build folder

Current Behavior after this fix:
- build no longer concatenates the vendor CSS while it now has correct,
standalone reference in index.html to vendor CSS copied to its vendor
folder
- compile has always vendor CSS concatenated (merged) and integrated
regardless of whether build task has been performed

Please kindly notice:
- watch on LESS change will not notify to concatenate vendor CSS because
they are not relevant in build
- Regardless of what's mentioned in the documentation, compile task
still partially depends on build task for javascript files in src/ while
all vendor files (incl. CSS, JS) can be compiled successfully all the
time. This is due to historical reasons that 'ngAnnotate' and
'concat:compile_js' loads file from build folder. As I'm quite unsure if
there exists modification during the process of copying those relevant
app js from original src to build, I just leave it remained as for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant