bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file
563 stars 71 forks source link

use resolve / include paths #5

Closed ccorcos closed 8 years ago

ccorcos commented 8 years ago

The search will continue while within the project directory and until a package.json or bower.json file is encountered.

I'm having an issue because my file structure looks like this currently

/proj1/image.png
/package.json
/proj2/package.json
/proj2/main.scss

I'm trying to require the image.png file from the main.scss file, but it cannot be resolved. My webpack config looks like this:

  module: {
    loaders: [
      { test: /\.js$/, loader: "babel" },
      { test: /\.coffee$/, loader: "babel!coffee" },
      { test: /\.scss$/, loader: "style!css!resolve-url!sass" },
      { test: /\.(svg|png|jpe?g|ttf|woff2?|eot)$/, loader: 'url?limit=8182' }
    ]
  },
  sassLoader: {
    includePaths: [
      path.resolve(".."),
      path.resolve("node_modules")
    ]
  },
  resolve: {
    root: [
      path.resolve('..'),
    ],
    modulesDirectories: [
      'node_modules'
    ]
  }

It would be sweet if this package actually looked at those resolve paths rather than crawl up to package.json

bholloway commented 8 years ago

Ok. Interesting use case.

Can you please attach the URL() statement that the sass file is using.

ccorcos commented 8 years ago
// proj2/main.scss
.something {
  background-image: url(proj1/image.png)
}

if I do this, it works:

.something {
  background-image: url(../proj1/image.png)
}

but this is still an issue because I @import scss files from proj1 as well with a similar absolute url scheme

bholloway commented 8 years ago

@ccorcos Webpack needs to be aware of proj1 as a module location.

I do not understand this aspect very well but refer to this and cherry-pick what you need. This is a general solution and you can probably remove the sassLoader.includePaths that you have.

Once you have that you can use the tilde (~) syntax to refer to the PNG asset and when you @import your (s)css files.

.something {
  background-image: url(~proj1/image.png)
}

Webpack is now fully aware of this asset (not just the sass loader) and can do all sorts of way cool processing on it.

Note that if you can't edit the Sass file then use my omit-tilde-webpack-plugin@0.5 to negate the need for the ~. However this is normally for migration purposes or to get you out of a bind. While I agree that the ~ is non-intuitive it is best practice and avoids path ambiguity in the (s)css.

Also note that the config I directed you to can source from either node_modules or bower_components. I setup your use case in that project it worked ok.

Let me know where this puts you.

ccorcos commented 8 years ago

Hmm. So at my job, we have a grunt / browserify system set up and I want to slowly merge it over to webpack. So this scss code is being use by both systems. Thus using the ~ isnt going to work for me right now... But that package looks interesting. Thanks for the help. I'll get back to you on this! :+1:

bholloway commented 8 years ago

Cool. I'm in exactly the same position. You will certainly want the omit-tilde plugin then.

ccorcos commented 8 years ago

Alright -- I dont really get this example in the REAMDE:

    new OmitTildeWebpackPlugin({
      test     : /\.someExt$/ ,
      include  : ['package.json', 'bower.json', 'some-package-name'],
      exclude  : 'some-other-package-name',
      deprecate: true
    }),

what extension? Should this be .scss? And I'm not sure what include means. Shouldn't it just look at the resolve config?

  resolve: {
    root: [
      path.resolve('..'),
    ],
    modulesDirectories: [
      'node_modules'
    ]
  }
ccorcos commented 8 years ago

Ah. I just realized I was missing one big thing in the explanation of this issue:

/proj1/image.png
/package.json
/proj2/package.json
/proj2/main.scss
/proj2/webpack.config.js  <-- webpack is running from here!

I still havent been able to get this to work even with your omit tilde plugin. I cant get webpack and SASS to register the root path as the previous directory

sassLoader: {
    includePaths: [
      path.resolve(".."),
      path.resolve("node_modules")
    ]
  },
  resolve: {
    root: [
      // path.resolve('.'),
      path.resolve('..'),
    ],
    modulesDirectories: [
      'node_modules'
    ]
  }
ccorcos commented 8 years ago

I keep running into errors like this:

ERROR in ./~/css-loader!./~/resolve-url-loader!./~/sass-loader!../checkout/src/styles/main.scss
Module not found: Error: Cannot resolve 'file' or 'directory' ./affirm-assets/src/images/check_50x50.svg in /Users/chetcorcos/code/web-ux/checkout/src/styles
 @ ./~/css-loader!./~/resolve-url-loader!./~/sass-loader!../checkout/src/styles/main.scss 6:9412-9465

My file structure is this:

 /Users/chetcorcos/code/web-ux/checkout/...
 /Users/chetcorcos/code/web-ux/portal/webpack.config.js

And I want all paths relative to /Users/chetcorcos/code/web-ux/.

ccorcos commented 8 years ago

Seems like this could be a one line fix to concat the resolve.root to the list of basePaths.

bholloway commented 8 years ago

Hey @ccorcos let me think out loud some of the options here.

Bearing in mind that these are in no particular order and some or all you may have already dismissed as unsuitable.

1. Make a Github fork and try this concat of the resolve.root and see where that gets you.

So in your Webpack project your package.json entry is resolve-url-router: ccorcos/resolve-url-router. Otherwise just npm link to a working copy of your fork.

That should get you moving on your migration, which I know from experience to be a frustrating issue.

However I think that you will have problems with a deploy build because Webpack will not be aware of all the assets that resolve-url-router has worked on. And ultimately you may be unhappy with this solution.

For my part I would need to see that this solution works for you, and whether it breaks any best practice, before moving forward in this direction.

2. You define a package.json for each of your dependencies (hopefully they are in seperate repositories) and install them as private npm dependencies.

For example, at my work we use private bitbucket repos and you will see lots of this sort of dep (full path omitted).

"angular-channel-logger": "git+ssh://git@bitbucket.org/.../angular-channel-logger.git#1.1.0"

This will give you a more conventional dependency structure. Your team will need ssh keys for the repo installed in their Git client to avoid having to enter a password every time they npm install.

Our old code base was a monolithic svn repository and this just would not have worked. If this is the case at your work then start lobbying for Git now because otherwise the current issue will be the least of your troubles.

3. Solve the problem using tilde then work out how omit-tilde-webpack-plugin can omit it.

I actually think you are pretty close.

I have updated the omit-tilde-webpack-plugin usage docs a little but I would first of all try to get it working with the tilde and then I can support you in omitting the tilde.

When I recreated your scenario I was able to make it work by specifying proj1 explicitly

new OmitTildePlugin({
  include  : 'proj1',
  deprecate: true
}),

There may be scope to improve automatic dependency identification using the resolve.root like you said. However I always worry when referring to the options directly from a loader/plugin. I have done it before but I believe it is a brittle approach.

ccorcos commented 8 years ago
  1. if webpack doenst recognize the files, then that kind of defeats the purpose of webpack :/
  2. not a bad idea. I could just link all the dependencies locally anyways right? Kind of a hack though...
  3. I'm not sure that will work because the resolve-url-loader only goes up to the latest package.json which will be within proj2. And the base path I want to target is not proj1 or proj2 but the parent of both of them...
everything/package.json

/everything/proj1/package.json
/everything/proj1/image.png

/everything/proj2/package.json
/everything/proj2/main.scss

/everything/proj2/webpack.config.js

in /everything/proj2/main.scss, I'm specifying background-image: url(proj1/image.png). Resolve-url-loader doesnt look outside proj2 because theres a package.json in proj2.

come to think of it, does webapck support tilde's on css urls?

bholloway commented 8 years ago

I tried 3 with a sass @import.

I'll check the behaviour with a URL() when I get back from lunch but my understanding is that webpack intercepts and then manages that asset reference. It should not hit the loader. I could be completely wrong.

ccorcos commented 8 years ago

I thought the whole point of this package was for resolving urls in SASS because the loader only handles paths relative to the top level importer

bholloway commented 8 years ago

Ok so having thought about it:

  1. css transpiler does its thing and spits out css.
  2. resolve-url-loader tries to amend the url() paths. Where there is a tilde then absolute === null per findFile.absolute() (could do with some optimisation). Where there is no tilde then it is legal as a relative path and findFile.absolute() quite rightly tries to find it. In both cases it should leave the url() as-is.
  3. css-loader takes any url() is sees and tries to resolve the path. The presence of the tilde denotes a module path and this is where your resolve.root comes in. If you are using the omit-tilde-webpack-plugin then it will intercept the resolve of relative paths and add a tilde if they match one of the names it was configured with.

So overall the change you proposed (option 1) would work all the way. However it duplicates the Webpack resolver mechanism and would bypass any custom resolver that is being used. So option 3 is still preferable.

I see what you mean about option 2 being a hack and with npm link it would be. However (without fully knowing your situation) it is probably best to aim long term at managing dependencies in separate repositories using npm. This is the expectation with node based build tools and their CI infrastructure and it certainly works very nicely.

bholloway commented 8 years ago

By the way I tested with .png and it is all good. I think you are very close.

ccorcos commented 8 years ago

my understanding is that webpack intercepts and then manages that asset reference. It should not hit the loader. I could be completely wrong.

It deosnt necessarily hit the loader, but it recognizes it as a resource and compiles that file to be distributed.

Using npm isnt ideal -- the goal is to bring these projects together -- not have circular dependencies between them...

ccorcos commented 8 years ago

ok. lets knock this out: https://github.com/ccorcos/webpack-sass-url-issue

bholloway commented 8 years ago

Cool. Will be online in a couple of hours.

ccorcos commented 8 years ago

awesome. Thanks dude! I really appreciate the help :)

bholloway commented 8 years ago

Check out this PR.

I'm still working on the root cause. But give that a try with your proper project and let me know if it also works.

bholloway commented 8 years ago

Found the root cause in this PR.

bholloway commented 8 years ago

Hey @ccorcos are you confident enough with your migration that we can close this issue yet?

ccorcos commented 8 years ago

Yeah, everything seems to be working for me now :+1: Thanks for all the help!

ccorcos commented 8 years ago

@bholloway curious, have you figured out how to do unit tests with angular + react? I'm getting errors in my angular controllers when I try to render to the dom.

bholloway commented 8 years ago

@ccorcos I have not gotten into react yet. Just transitioning Browserify to Webpack for Angular builds at this stage.

However I was inspired to do so after a meetup where the presenter showed the sort of Angular-React components you speak of. So I'm sure there is a solution but none that I have experience with.

ccorcos commented 8 years ago

OK. Thanks! I may have figured it out -- using a directive... I'm not too familiar with angular though.