webpack-contrib / file-loader

File Loader
MIT License
1.86k stars 257 forks source link

revert(index): `context` takes precedence over `issuer.context` (`options.useRelativePath`) #260

Closed michael-ciniawsky closed 6 years ago

michael-ciniawsky commented 6 years ago

Issues

alexander-akait commented 6 years ago

@michael-ciniawsky need ping someone who have this bug and tests this branch

zemadz commented 6 years ago

I'm afraid it does not restore the behavior that was in 1.1.7. Tested with issue reproduction repository (https://github.com/zemadz/file-loader-relative-path-minimum-repro) and my actual project that uses file-loader for CSS generation. I tried many combinations but were unable to achieve desired output with this branch @aea46b8b94b15e53e38761e0109850868c7a7b2d.

michael-ciniawsky commented 6 years ago

@zemadz The following seems to work

{
  test: /\.(png)$/,
  use: [
    {
      loader: 'file-loader',
      options: {
        name: '[name].[ext]',
        // outputPath: paths.build,
        useRelativePath: true
      }
    }
  ]
}
assets/image.png
js/index.js
{
  test: /\.(png)$/,
  use: [
    {
      loader: 'file-loader',
      options: {
        name: '[name].[ext]',
        outputPath (url) {
          return 'test/' + url
        }
        useRelativePath: true
      }
    }
  ]
}
test/assets/image.png
js/index.js

Your're getting something like this with this patch applied atm right?

/Users/User/Github/file-loader-relative-path-minimum-repro/build/assets/image.png        
js/index.js
zemadz commented 6 years ago

With config:

{
  test: /\.(png)$/,
  use: [{
    loader: 'file-loader',
    options: {
      name: '[name].[ext]',
      // outputPath: paths.build,
      useRelativePath: true
    }
  }]
}

I get the expected files at build/assets/image.png and build/js/index.js, where the javascript exports module.exports = __webpack_require__.p + "assets/image.png";. I expect the export to equal module.exports = __webpack_require__.p + "../assets/image.png" since that would be the relative path to the image from the js file.

With the current pull request the following produces exactly the same output as the previous config:

{
  test: /\.(png)$/,
  use: [{
    loader: 'file-loader',
    options: {
      name: '[path][name].[ext]',
      // outputPath: paths.build,
      // useRelativePath: true
    }
  }]
}
michael-ciniawsky commented 6 years ago

@zemadz The export points to the destination of the outputted file ? Can you explain the use case or how this currently breaks an application of yours ?

file loader

zemadz commented 6 years ago

I'm using Webpack for compiling SASS. The idea is that there is a scss directory for sources which will have matching outputs at css directory when compiled. Assets are not handled by Webpack bundle, but they can be referenced from the styles.

I've made an example with my use case (https://github.com/zemadz/file-loader-relative-path-css-repro). There are three branches master (1.1.8), working (1.1.7), revert (file-loader revert branch). I created a test that can be run with npm test to describe the expected output.

michael-ciniawsky commented 6 years ago

This PR fixes the direct regression introduced in https://github.com/webpack-contrib/file-loader/commit/3b071f5279ef6cb912d66560e38aa9bbe7214c30 @zemadz I will take checkout your second example soon, but due to extract-text-webpack-plugin && resolve-url-loader involved I'm pretty sure your particular issue lies elsewhere, there are known issues with this setup in regard to file path resolving