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

Resolve function behavior #216

Closed
cookpete opened this issue Jun 6, 2016 · 14 comments
Closed

Resolve function behavior #216

cookpete opened this issue Jun 6, 2016 · 14 comments
Assignees
Milestone

Comments

@cookpete
Copy link

cookpete commented Jun 6, 2016

This is a slightly tricky problem to explain so bear with me.

I am using resolve to enable some sort of "theme" system for my stylesheets. I am using a fairly simple resolve function:

postcssImport({
  addDependencyTo: webpack,
  resolve: id => id.replace(/^theme/, PATH_THEME)
})

So now I can do:

@import 'theme/variables.css';

and this will, for example, give me the variables from src/themes/default/variables.css for my default theme. Great.

The problem happens when another imported file tries to import variables.css. For example, I add a mixins.css file with:

@import 'theme/variables.css';
/* Use variables here for mixins */

and then I want to use

@import './mixins.css';

in my stylesheets, but it seems like the resolve rule is no longer kicking in for the deeper @import. Now I get the error:

client?843a:47 ./~/css-loader?modules&sourceMap&localIdentName=[hash:base64:3]!./~/postcss-loader!./src/components/Navigation.css
Module build failed: Error: ENOENT: no such file or directory, open '/Users/user/repo/src/theme/variables.css'
    at Error (native)
 @ ./src/components/Navigation.css 4:14-178 13:2-17:4 14:20-184

You can see it's trying to open theme/variables.css which doesn't exist and isn't being resolveed according to the configuration.

Am I doing something wrong, or is this a bug/limitation of the resolve feature?

@MoOx
Copy link
Contributor

MoOx commented Jun 6, 2016

This may seems like a bug. But to be honest v8 is pretty buggy. See #210.

@cookpete
Copy link
Author

cookpete commented Jun 7, 2016

Ok so I dug around a bit and the default resolve function was no longer kicking in for files that did not match my regex. I have changed my code to:

import postcssImport from 'postcss-import'
import postcssImportResolve from 'postcss-import/lib/resolve-id'

// ...

postcssImport({
  addDependencyTo: webpack,
  resolve: (id, base, options) => {
    if (/^theme/.test(id)) {
      return id.replace(/^theme/, PATH_THEME)
    } else {
      return postcssImportResolve(id, base, options)
    }
  }
})

and it seems to work now, although it is a rather ugly fix. I wonder if it's worth adding a check around where the resolve happens and apply the default resolveId function if the passed in path does not change? Or some other way of knowing when the default function should kick in?

@MoOx
Copy link
Contributor

MoOx commented Jun 7, 2016

Please consider this comment #169 (comment)

@nicotsou
Copy link

nicotsou commented Jul 5, 2016

I was able to resolve this issue by using the plugin postcss-advanced-variables. I just placed it at the end of the plugins list for postcss in webpack configuration and it worked. Not sure why!

@MoOx
Copy link
Contributor

MoOx commented Jul 5, 2016

Probably luck. Not sure it will work all the time on the long run...

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 3, 2016

@cookpete I believe this is intended behavior. From the docs:

You can overwrite the default path resolving way by setting this option. This function gets (id, basedir, importOptions) arguments and returns full path, array of paths or promise resolving paths.

However, I agree that your use-case requires ugly workarounds.

I wonder if it's worth adding a check around where the resolve happens and apply the default resolveId function if the passed in path does not change?

Perhaps; I don't know if this is a good idea or not.

I don't have time to work on/think about this ATM. If you make a PR, I will review it, though.

@RyanZim RyanZim changed the title Resolve is not kicking in with nested imports Resolve function behavior Nov 3, 2016
@RyanZim
Copy link
Collaborator

RyanZim commented Nov 18, 2016

@ai @MoOx Thoughts on this issue?

@ai
Copy link
Member

ai commented Nov 18, 2016

@RyanZim what is the question? :)

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 19, 2016

If someone passes a custom resolve function, and that function returns the path without resolving it, should postcss-import use the default resolver on it?

@ai
Copy link
Member

ai commented Nov 19, 2016

Why not? isAbsolute method is in the path module and easy to use.

User show real useful case.

In my opinion we should care about users if it is not dramatically reduce maintainability (and we have no reduce here).

(Thanks for explanation)

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 19, 2016

So you would be in favor of adding this?

@ai
Copy link
Member

ai commented Nov 19, 2016

Yeap. Small nice sugar.

@RyanZim RyanZim added this to the v9.0.0 milestone Nov 19, 2016
@RyanZim RyanZim self-assigned this Nov 19, 2016
@RyanZim
Copy link
Collaborator

RyanZim commented Nov 22, 2016

This was changed in #249, see details there.

@RyanZim RyanZim closed this as completed Nov 22, 2016
@ahahn95
Copy link

ahahn95 commented Mar 28, 2019

Apologies for resurrecting this, but @cookpete, what was your PATH_THEME var equal to? I'm having a hard time getting this to work properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants