Skip to content

Webpack v4 rc7 images wrong pack reference #1915

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

Closed
JanStevens opened this issue Jan 25, 2019 · 11 comments
Closed

Webpack v4 rc7 images wrong pack reference #1915

JanStevens opened this issue Jan 25, 2019 · 11 comments

Comments

@JanStevens
Copy link

Hello,

We are following every RC update so today I upgraded from rc5 to rc7 and I noticed that all my images now need to be references differently. To get started some configuration files:

default: &default
  source_path: app/javascript
  source_entry_path: packs
  public_root_path: public
  public_output_path: packs
  cache_path: tmp/cache/webpacker
  check_yarn_integrity: false
  webpack_compile_output: false

  # Additional paths webpack should lookup modules
  # ['app/assets', 'engine/foo/app/assets']
  resolved_paths: ['app/javascript/src']

  # Reload manifest.json on all requests so we reload latest compiled packs
  cache_manifest: false

  # Extract CSS files
  extract_css: true
  
  static_assets_extensions:
    - .jpg
    - .jpeg
    - .png
    - .gif
    - .tiff
    - .ico
    - .svg
    - .eot
    - .otf
    - .ttf
    - .woff
    - .woff2

  extensions:
    - .mjs
    - .jsx
    - .js
    - .sass
    - .scss
    - .css
    - .module.sass
    - .module.scss
    - .module.css
    - .png
    - .svg
    - .gif
    - .jpeg
    - .jpg
    - .graphql

development:
  <<: *default
  compile: true

  check_yarn_integrity: true

In our main application.js file we require the images (else they would never be included in the pack)

import 'bootstrap'
// Start Rails UJS
import Rails from 'rails-ujs'
Rails.start()
// Require images
require.context('../images/', true, /\.(gif|jpg|png|svg|ico)$/i)

Our environment.js didn't change, we do add the sass resolve-url-loader

environment.loaders.get('sass').use.splice(-1, 0, {
  loader: 'resolve-url-loader'
})

For example the favicon in RC5 could be referenced as:

  <link rel="shortcut icon" href="<%= asset_pack_path('images/favicon.ico') %>" data-environment="<%= Rails.env %>">

But in RC7 this needs to be

<link rel="shortcut icon" href="<%= asset_pack_path('media/favicon.ico') %>" data-environment="<%= Rails.env %>">

Even more problematic are subdirectories in the images (for example app/javascript/images/logo/brand-color.png), in RC5

<%= image_pack_tag('images/logo/brand-color.png', class: 'd-inline-block align-top') %>

In RC7 this got changed again to media but it also removed the subdir

<%= image_pack_tag('media/brand-color.png', class: 'd-inline-block align-top') %>

So I'm wondering how I should reference images correctly and if that subdirectory is not a bug or something.

Regards

@gauravtiwari
Copy link
Member

gauravtiwari commented Jan 25, 2019

Hi, @JanStevens Earlier, we had full path embedded relative to the context for static assets, however, to simplify this I changed it just file name and hash. That media folder is just a namespace for static assets so you would need it before calling any static file through webpacker. And calling images are much simpler now, no nested sub-directories - media/{pack_name}

 "media/google.jpg": "/packs/media/google-97e897b3.jpg",
 "media/rails.png": "/packs/media/rails-45b116b1.png",

screenshot 2019-01-25 at 15 12 01

screenshot 2019-01-25 at 15 12 07

You would notice, everything is now under the correct directory, inside packs folder for simplicity instead of dumping everything in the packs folder.

@gauravtiwari
Copy link
Member

Here is bit more background on the earlier issue: https://github.com/rails/webpacker/blob/master/CHANGELOG.md#fixed-1

@JanStevens
Copy link
Author

Hello,

Yea I found that link (stupid should first check the Changelog ofcourse), I'm just a bit concerned that flatting it out would cause some naming issues if I have two files name exactly the same but in a sub directory, example:

images/logo/company.png
images/company.png

This would result in one company.png image instead of two (and the logo is a smaller one for example)

@gauravtiwari
Copy link
Member

gauravtiwari commented Jan 25, 2019

Yes of course, I know.

I guess that was the reason for earlier change and adding context option in the first place but it seemed it caused more harm especially, for assets inside node_modules or app/assets, which has really weird paths like - /packs/_/assets/images/avatar-057862c747f0fdbeae506bdd0516cad1.png or /packs/_/_/node_modules/bootstrap/icons-057862c747f0fdbeae506bdd0516cad1.svg

I guess a simple fix for this would be to name images correctly instead of directory namespacing - company_logo or company-logo and company.png and put them in one folder.

Obviously, if you don't want the behaviour you can update file loader to include context option but it seems not having is much better generally,

const { environment, config } = require('@rails/webpacker')
const { join } = require('path')

const fileLoader = environment.loaders.get('file')
fileLoader.use[0].options.context = join(config.source_path)

@gauravtiwari
Copy link
Member

Sorry, this came as bit of surprise but we all learn :)

@JanStevens
Copy link
Author

Oke that makes it clear, I think some added documentation in the README might be handy as also a guide on how to add images in your pack with webpack.

Thanks for the fast reply!

@gauravtiwari
Copy link
Member

Sure, documentation needs overhauling a bit. If you have some time, please make a PR and I will merge.

@meneerprins
Copy link

Hi,

Is there a way to make it work like the old way? The above mentioned solution doesn't work for me.

Our whole app is namespaced (like frontend/app1/images, frontend/app2/images, frontend/app3/images)

and each app had it's own entrypoint with
require.context('app1/images', true, /\.(gif|jpg|png|svg)$/i);

but with the new update nothing works anymore, which needs a refactor of the entire image import system with hundreds of image references...

any solution?

@gauravtiwari
Copy link
Member

gauravtiwari commented Jan 25, 2019

Any error you getting?

const { environment, config } = require('@rails/webpacker')
const { join } = require('path')

const fileLoader = environment.loaders.get('file')
fileLoader.use[0].options.context = join(config.source_path)

https://github.com/webpack-contrib/file-loader#context (need to set this variable to app source)

@meneerprins
Copy link

Well I am probably doing something wrong because I assumed I could just paste that code in environment.js but many variables are undefined and webpack 'cannot find @rails/webpacker/config'. I'm not that familiar with advanced webpack configs, could you maybe give a more extended example?

@gauravtiwari
Copy link
Member

Updated the example, just wrote off the top of my head. My bad :)

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

No branches or pull requests

3 participants