sbt / sbt-rjs

RequireJs optimizer plugin for sbt-web
Other
39 stars 28 forks source link

Windows fix regarding paths #26

Closed DGolubets closed 10 years ago

DGolubets commented 10 years ago

I tested it on my Win 8.1 None of sample projects worked correctly on my PC. When I started digging I found that a lot of code (both Scala and JS) relies on / path separators whilst windows gives . And pull https://github.com/sbt/sbt-rjs/pull/17 just fixed error message but not the problem.

My solution is to replace \ with /. Windows understands such paths too and other code can just rely on one path separator.

I've done replacement where was needed. Few paths in app.build.js are still generated with backslashes, but as far as I tested they don't break anything.

huntc commented 10 years ago

I'd prefer to retain the windows path sep where required. Perhaps the issue here is that we are using a windows sep where a uri path sep is required and a unix sep where a windows sep is required. What's the actual error you're seeing... I did have this working on Windows at one point...

DGolubets commented 10 years ago

Hi! The project is using webjars: requirejs, angular, etc. Sbt-rjs as I understand should do 2 things for them: 1) replace their paths with CND in main.js 2) set them to "empty:" in app.build.js I looked into generated app.build.js and there were no such changes.

Here is an example of my paths:

 paths: {
      'requirejs': '../lib/requirejs/require',
      'jquery': '../lib/jquery/jquery',
      'angular': '../lib/angularjs/angular'
    }

They all are defined with forward slash and this seems correct cos they are url parts indeed.

First of all regexp in getWebJarPaths failed to match them. When I run show rjs-paths I get nothing. That's cos I have "lib\" path in Windows.

Then, when I fixed the expression, "empty:" paths were added, but not CDNs. buildWriter.js doesn't take into account different path separators. It simply compares paths like endsWith(path, mainConfigFile) and when they use different slashes the check fails.

And an important note: node seems to use unix path separator even on windows. That's what I see in logs during a build:

[info] Uglify2 file: E:/Dev/Projects/temp/pwar23/play-angular-require-seed/target/web/rjs/build/lib/underscorejs/underscore.js
huntc commented 10 years ago

Hi @DGolubets,

Thank you for your contribution. Would you mind signing our CLA?

http://www.typesafe.com/contribute/cla

Thanks!

huntc commented 10 years ago

@pvlugter WDYT?

alex-vas commented 10 years ago

May be this fix could warrant another release? Cheers.

huntc commented 10 years ago

Sure, I will make another release once this has been merged.

DGolubets commented 10 years ago

Hi! Signed.

huntc commented 10 years ago

Released version 1.0.3. Thanks for the contributions everyone!