Skip to content

Conversation

@dchekanov
Copy link

Fixes #207.

@dingus9
Copy link

dingus9 commented May 17, 2016

This fix works for me please prioritize a merge

@alundiak
Copy link
Collaborator

alundiak commented Feb 18, 2017

@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
Copy link
Author

@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
Copy link
Collaborator

alundiak commented Feb 23, 2017

@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.

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.

3 participants