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

Fix windows separators in pathStr #3

Closed
wants to merge 1 commit into from
Closed

Fix windows separators in pathStr #3

wants to merge 1 commit into from

Conversation

Guria
Copy link

@Guria Guria commented Aug 16, 2017

follow up from bem/webpack-bem-loader#26

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 494a148 on Guria:patch-1 into 17da371 on Yeti-or:master.

@Guria
Copy link
Author

Guria commented Aug 16, 2017

hmm. This will fix an issue but seems problem is not in require and/or webpack. investigating.

@Guria
Copy link
Author

Guria commented Aug 16, 2017

@@ -1,6 +1,7 @@
var path = require('path');
var assert = require('assert');

// path.sep shouldn't be used since require doesn't support windows separator
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment statement isn't true.
see bem/webpack-bem-loader#26 (comment)

@Guria
Copy link
Author

Guria commented Aug 16, 2017

@Yeti-or How about a keepSeparator flag? Do not replace anything and use path.sep when making relative if flag is set to true or replace any windows separators with nix ones otherwise?

@Yeti-or
Copy link
Owner

Yeti-or commented Aug 16, 2017 via email

@Guria
Copy link
Author

Guria commented Aug 18, 2017

@Yeti-or Did you come up with smth?

@tadatuta
Copy link

@Yeti-or ping!

@Guria
Copy link
Author

Guria commented Sep 3, 2017

would be great to hear smth here so we would be able to move on

@Yeti-or
Copy link
Owner

Yeti-or commented Dec 6, 2017

We fixed it inside webpack-bem-loader it's not a problem with required-path

@Yeti-or Yeti-or closed this Dec 6, 2017
@Yeti-or
Copy link
Owner

Yeti-or commented Dec 6, 2017

@Guria check bem/webpack-bem-loader#52
I am very sorry for such a long delay 😔

@Guria Guria deleted the patch-1 branch December 6, 2017 13:09
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.

4 participants