guybedford / require-css

A RequireJS CSS loader plugin to allow CSS requires and optimization
MIT License
983 stars 364 forks source link

Exclude external resources from r.js optimization #208

Open dchekanov opened 8 years ago

dchekanov commented 8 years ago

Fixes #207.

dingus9 commented 8 years ago

This fix works for me please prioritize a merge

alundiak commented 7 years ago

@dchekanov I reviewed this PR a bit, and I will do more deep research how it might affect end customers. Meanwhile, if u still available for this module, could u please think on test case to write in test.js (or let me know what exactly test result u relied on).

If no reply during the week, I will recreate PR from my own with suggested code changes.

cc/ @dingus9 @linjinying @amenadiel

dchekanov commented 7 years ago

@alundiak this plugin currently doesn't have any tests for the building part, so I'm not sure how the suggested change should be tested.

There are 2 things that this PR does to support loading modules via the paths directive:

  1. When loading a module, it checks for the final module location ('https://domain.com/file.js') and not for its name ('moduleName') to be an absolute URL.
  2. When writing module contents, it checks if the module has been loaded and not if its name is an absolute URL.
alundiak commented 7 years ago

@dchekanov 1) Test case 1: run command r.js -o example/build.js 2) Test case 2: edit example/build.js => optimizeCss => "standard" and run above command again. 3) Would also great to know how buildCSS and separateCSS change affects build.

First 2 will show us at least, if all files (css.js, css-builder.js, normalize.js) working file.

And I will think about this steps later and provide pr with test for such test cases in test.js.