andywer/postcss-theme

themePath - conversion of forward slash to back slash causes incompatibility on Windows OS

Closed this issue · 12 comments

In a windows environment, using webpack.

For the given configuration:

postcss: function () {
    return [
      theme({ themePath: 'c:/project/ui/src/styles/themes/default'}),
      values,
      autoprefixer
    ]
  }

When used with the following in line in css:

@value tooltip-text from theme(themecolors);

The module is resolved to the following directory:

c:\project\ui\src\styles\themes\default\themecolors.css

Which can't be resolved by windows during compilation.

Module build failed: ModuleNotFoundError: Module not found: Error: Cannot resolve module '"c:\project\ui\src\styles\themes\default\themecolors.css"'

As a test - Hard-coding the path in the css file and using forward slashes works

@value tooltip-text from 'c:/project/ui/src/styles/themes/default';

Hi @lordy81. What path do you expect instead?

Looks like everything is working fine. The import from the hard-coded path c:/project/ui/src/styles/themes/default seems odd. Shouldn't it be c:/project/ui/src/styles/themes/default/some-file.css?

Thanks for the quick reply.

Instead of:

c:\project\ui\src\styles\themes\default\themecolors.css

The resolved path needs to be:

c:/project/ui/src/styles/themes/default/themecolors.css

Sorry, you're right I got the hard-coded example wrong - but that was just for illustration.

I've narrowed down the transition to where it occurs in themeFileResolver.

It seems path.join is the cause - it is switching the slash direction

path.sep is set to \.

I'll take a look at the path doc to see if this can be changed.

Added logs as follows (Line 22 of index.js in the version I have)

/**
 * @param {object} options { themePath: String }
 */
module.exports = postcss.plugin('postcss-theme', function (options) {
  var themeFileResolver = function (themeFilePath) {
    if (!themeFilePath.match(/\.css$/i)) {
      themeFilePath += '.css'
    }

    console.log("In themeFileResolver");
    console.log("themePath: " + options.themePath);
    console.log("themeFilePath: " + themeFilePath);
    console.log("joined: " + path.join(options.themePath, themeFilePath));

    return path.join(options.themePath, themeFilePath);
  }

Gives this output:

In themeFileResolver
themePath: c:/project/ui/src/styles/themes/default
themeFilePath: themecolors.css
joined: c:\project\ui\src\styles\themes\default\themecolors.css

Was looking for uses of path.sep in the other node modules in the project - Have noticed a few which track path.sep and alter it for windows usage as follows:

    // windows support: need to use /, not \
    if (path.sep !== '/') {
      pattern = pattern.split(path.sep).join('/')
    }

Modified themeFileResolver as follows

/**
 * @param {object} options { themePath: String }
 */
module.exports = postcss.plugin('postcss-theme', function (options) {
  var themeFileResolver = function (themeFilePath) {
    if (!themeFilePath.match(/\.css$/i)) {
      themeFilePath += '.css'
    }

    var themeFile = path.join(options.themePath, themeFilePath)
    // windows support: need to use /, not \
    if (path.sep !== '/') {
      themeFile = themeFile.split(path.sep).join('/')
    }
    return themeFile;
  }

Yeah, I've also noticed that it is path.join. Unfortunately I won't have time to fix anything before evening (in a few hours).

But I have one question: Do you really need the slashes? I mean backslashes in paths when using Windows are rather a feature than a bug ;)

Save you the time, I've forked the project and added a pull request - let me know what you think.

Its a pain, but we need the forward slash otherwise it won't compile.

fs.stat (used by ModuleAsDirectoryPlugin.js) won't recognise the file with single backslashes.

The comments in other node modules I've looked at say they've made the changes because functionality like 'require' expects a forward slash.

Ahhh. Actually wanted to close this issue, but closed the PR instead... 😅

Have a look at version 1.0.1. Tell me if it works for you!

Ah, I can see what you've done, and that's great. 👍

However - The other issue is that in order to simplify the description of the issue I used a hard-coded string for the themePath. In reality we use path.join as well in the config, which converts to backslashes before it reaches your code.

Personally, I'd prefer the change all instances approach - this would also mean that other developers can use either \ or / in the config.

Hmm... I can totally understand your motivation, but I think this is not the right place to fix this.

But... I am implementing a minor new functionality anyway and maybe you can use that to fix your problem. It will be possible to optionally pass a custom themeFileResolver function as an option. It's basically just a function that replaces the themeFileResolver function in index.js.

Yours could look like that:

function themeFileResolver (themeFilePath, options, originalResolver) {
  return originalResolver(themeFilePath, options).replace(/\\/g, '/')
}

...

postcssTheme({ themePath: './some/path', themeFileResolver })

What do you think?

That would work. Brill. I'll watch the project for the update.

Any ETA?

Let's say in about 4h.

Released version 1.1.0