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

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

Open
gimre opened this issue Jan 12, 2016 · 21 comments
Open

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

gimre opened this issue Jan 12, 2016 · 21 comments

Comments

@gimre
Copy link

gimre commented Jan 12, 2016

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:

  • header.css is being imported from a react component as part of a browserify stack
  • both css files are in the same directory
  • the same thing happens with any compose statement

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

gimre commented Jan 12, 2016

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

@tivac
Copy link

tivac commented Jan 12, 2016

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

gimre commented Jan 12, 2016

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

gimre commented Jan 18, 2016

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

@r-flash
Copy link

r-flash commented Mar 8, 2016

Not sure if it's a new information, but i have found a problem here

if (newPath[0] !== '.' && newPath[0] !== '/') {
. 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
Copy link
Member

joshwnj commented Mar 8, 2016

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

tivac commented Mar 8, 2016

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

joshwnj commented Mar 8, 2016

super helpful, thanks @tivac !

@r-flash
Copy link

r-flash commented Mar 8, 2016

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

@Nimelrian
Copy link

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

@joshwnj
Copy link
Member

joshwnj commented Jul 4, 2017

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
Copy link
Member

joshwnj commented Jul 4, 2017

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

Nimelrian commented Jul 4, 2017

@joshwnj See my comment in the PR.

@joshwnj
Copy link
Member

joshwnj commented Jul 4, 2017

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

joshwnj commented Jul 4, 2017

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

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

Nimelrian commented Jul 4, 2017

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

joshwnj commented Jul 8, 2017

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 :)

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

No branches or pull requests

6 participants