css-modules / css-modulesify

A browserify plugin to load CSS Modules
MIT License
403 stars 47 forks source link

composes (import) doesn't seem to work at all (on Windows at least) #74

Open gimre opened 8 years ago

gimre commented 8 years ago

Hey, trying to use a simple import statement to import a value from another css file:

values.css

@value someValue: 500px;

header.css

@value someValue from './values.css';

:global( .header ) {
    background-color: someValue;
}

I'm getting the following error:

NO NODE D:\dev\browserify-react\D:\D:\src\components\header\styles.css D:\dev\browserify-react\D:\D:
\src\components\header\values.css
[Error: Node does not exist: D:\dev\browserify-react\D:\D:\src\components\header\styles.css]

Notes:

Any idea what is wrong here? :( I tried to investigate the file loading code, but I didn't have any luck understading what was going on.

gimre commented 8 years ago

Oh, if i just use a local value it works, no complaints.

tivac commented 8 years ago

D:\dev\browserify-react\D:\D:\src\components\header\styles.css

That path looks real broken, which is a problem I've run into before. I can't find the issue any more though :(

gimre commented 8 years ago

Yes, I noticed :( I didn't have enough time to debug the loader mechanism to see where it goes wrong - I had a hunch maybe it's Windows only problem. Anyone? :s

tjallingt commented 8 years ago

I have exactly the same issue with the :imports not working and the path's being broken... I might look into it later.

Experienced this issue on two windows environments... (both on the D:/ drive so it might be related to that)

D:\Github\css-modulesify>npm run test

> css-modulesify@0.16.1 test D:\Github\css-modulesify
> tape tests/*.js

TAP version 13
# multiple builds
ok 1 initial bundle without a cache does not error
ok 2 correct output filename
not ok 3 output matches expected
  ---
    operator: equal
    expected: |-
      '._styles__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      '._D_D_styles__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
ok 4 second pass bundle with a cache does not error
ok 5 correct output filename
not ok 6 output matches expected
  ---
    operator: equal
    expected: |-
      '._styles__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      '._D_D_styles__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
# case: compose-from-shared
NO NODE D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\styles-1.css D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\shared.css
[Error: Node does not exist: D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\styles-1.css]
NO NODE D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\styles-2.css D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\shared.css
[Error: Node does not exist: D:\Github\css-modulesify\tests\cases\compose-from-shared\D:\D:\styles-2.css]
ok 7 correct output filename
not ok 8 output matches expected
  ---
    operator: equal
    expected: |-
      '._shared__shared {\r\n  background: #000;\r\n}\r\n._styles_1__foo {\r\n  color: #F00;\r\n}\r\n._styles_2__bar {\r\n  background: #BAA;\r\n}\r\n'
    actual: |-
      ':import("./shared.css") {\n  i__imported_shared_0: shared;\n}\n._D_D_styles_1__foo {\r\n  color: #F00;\r\n}\r\n:import("./shared.css") {\n  i__imported_shared_0: shared;\n}\n._D_D_styles_2__bar {\r\n  background: #BAA;\r\n}\r\n'
  ...
# case: compose-node-module
NO NODE D:\Github\css-modulesify\tests\cases\compose-node-module\D:\D:\styles.css D:\Github\css-modulesify\node_modules\cool-styles\styles.css
[Error: Node does not exist: D:\Github\css-modulesify\tests\cases\compose-node-module\D:\D:\styles.css]
ok 9 correct output filename
not ok 10 output matches expected
  ---
    operator: equal
    expected: |-
      '._node_modules_cool_styles_styles__foo {\r\n  color: #F00;\r\n}\r\n._styles__foo {\r\n  background: black;\r\n}\r\n'
    actual: |-
      ':import("cool-styles/styles.css") {\n  i__imported_foo_0: foo;\n}\n._D_D_styles__foo {\r\n  background: black;\r\n}\r\n'
  ...
# case: import-and-compose
NO NODE D:\Github\css-modulesify\tests\cases\import-and-compose\D:\D:\styles-1.css D:\Github\css-modulesify\tests\cases\import-and-compose\D:\D:\styles-2.css
[Error: Node does not exist: D:\Github\css-modulesify\tests\cases\import-and-compose\D:\D:\styles-1.css]
ok 11 correct output filename
not ok 12 output matches expected
  ---
    operator: equal
    expected: |-
      '._styles_2__bar {\r\n  background: #BAA;\r\n}\r\n._styles_1__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      ':import("./styles-2.css") {\n  i__imported_bar_0: bar;\n}\n._D_D_styles_1__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
# case: import-node-module
ok 13 correct output filename
not ok 14 output matches expected
  ---
    operator: equal
    expected: |-
      '._node_modules_cool_styles_styles__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      '._D_D_node_modules_cool_styles_styles__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
# case: multiple-js-files
ok 15 correct output filename
not ok 16 output matches expected
  ---
    operator: equal
    expected: |-
      '._simple_styles__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      '._D_D_simple_styles__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
# case: simple
ok 17 correct output filename
not ok 18 output matches expected
  ---
    operator: equal
    expected: |-
      '._styles__foo {\r\n  color: #F00;\r\n}\r\n'
    actual: |-
      '._D_D_styles__foo {\r\n  color: #F00;\r\n}\r\n'
  ...
# stream output
ok 19 emits data for ._D_D_styles__foo
not ok 20 emits compiled css for ._D_D_styles__foo
  ---
    operator: ok
    expected: true
    actual:   false
  ...
ok 21 ends the stream

1..21
# tests 21
# pass  12
# fail  9

npm ERR! Windows_NT 10.0.10586
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\Tjalling\\AppData\\Roaming\\npm\\node_modules\\npm\\bin\\npm-cli.js" "run" "test"
npm ERR! node v5.2.0
npm ERR! npm  v3.5.1
npm ERR! code ELIFECYCLE
npm ERR! css-modulesify@0.16.1 test: `tape tests/*.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the css-modulesify@0.16.1 test script 'tape tests/*.js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the css-modulesify package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     tape tests/*.js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs css-modulesify
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls css-modulesify
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     D:\Github\css-modulesify\npm-debug.log
tjallingt commented 8 years ago

Tried looking into the issue but since I'm not quite clear on how all the components work together I can't seem to pinpoint the issue. I think it might be related to the rootRelativePath inside the fileSystemLoader as that variable contains very strange paths (such as D:\D:\styles-1.css) but when I try to change it to something more sane the tests stop working without giving me any errors that I can work from and I don't really understand what that value is for anyway. The path with the two D's also get passed to the generateScopedName functions which makes them behave weird (which explains the _D_D in front of all stylenames).

tjallingt commented 8 years ago

So after some looking around on the issues and pull requests pages I found out that this issue is probably related to #66, #56 and will be fixed by the pull request made on the loader-core here

Can we get a status update on the status of these changes?

gimre commented 8 years ago

Thanks for following up; hope this gets merged soon - I am sad :( :D

r-flash commented 8 years ago

Not sure if it's a new information, but i have found a problem here https://github.com/css-modules/css-modulesify/blob/f428deda84b2eda745f7dab233c86f8e99ebb6b0/file-system-loader.js#L72 . On Windows, absolute paths begin with a letter, so this test gives false positive;

The aformentioned loader-core pullrequest is going to have the same issue here https://github.com/sullenor/css-modules-loader-core/blob/b626256652eb1d658c5a4d039d5136eb23a26a6d/src/file-system-loader.js#L34 .

I believe this should be reported in the loader repo, or am i wrong? (I don't know anything about it, so i better ask :) )

joshwnj commented 8 years ago

Thanks for the tip @r-flash . Yes please report in the loader repo, that will be helpful.

Can you suggest a better approach for determining whether a path is relative or absolute that will work on windows?

tivac commented 8 years ago

@joshwnj path-is-absolute, or just using path.isAbsolute depending on the node level you support.

For modular-css I use resolve-from on every file path, because it'll do the right thing and follow the node conventions as expected in my experience (it uses the node require.resolve code under the covers).

joshwnj commented 8 years ago

super helpful, thanks @tivac !

r-flash commented 8 years ago

Just found out this was already taken care of in this PR.

Nimelrian commented 7 years ago

Any updates on this? Being unable to use one of the best features of CSS modules on a whole OS is kinda frustrating.

joshwnj commented 7 years ago

I think I can take a bit of time later today to put together a PR based on the suggestions in this thread (but if anyone else beats me to it that's cool too :P )

And then I'll ask for your help to test on windows @Nimelrian :) Thanks for your interest in getting this resolved

joshwnj commented 7 years ago

@Nimelrian when you have a moment could you please check whether #117 fixes the issue? There might be more to it, but this will at least fix one of the windows compatibility issues as pointed out by others in the thread above.

Nimelrian commented 7 years ago

@joshwnj See my comment in the PR.

joshwnj commented 7 years ago

@Nimelrian thanks for the notes in the PR. Would you also be able to create a small github repo with those files, so we can test against that failing case more easily?

joshwnj commented 7 years ago

I wonder if travis-ci (or equivalent) has an option to build on a windows machine? Anybody have an idea of how we could set up something like that?

Nimelrian commented 7 years ago

Test repo: https://github.com/Nimelrian/css-modulesify-windows-test

travis-ci can not build on Windows AFAIK.

Jenkins can for sure if you have the building node running on Windows.

Nimelrian commented 7 years ago

@joshwnj The problem definitely lies either in loader.fetch or the way you call it. I'm not sure if it is really correct to call fetch with '/' as the relativeTo parameter when the relative filename is not relative to '/' but to CMify's rootDir.

Edit: I think there's also a problem with the way relativeDir, rootRelativeDir, rootRelativePath, fileRelativePath in file-system-loader.js are initialized. The variables are named "relative" but their values are actually absolute.

joshwnj commented 7 years ago

Sorry I haven't been able to dig further into this. I'm OOO all next week, so been busy getting ready for that :)

If anyone else would like to step in and figure out the next piece of this, please feel free :)