techjacker / node-version-assets

Version your static assets with MD5 hashes using node.js
MIT License
63 stars 11 forks source link

CSS version replacement #7

Closed shanestillwell closed 7 years ago

shanestillwell commented 11 years ago

This might be a bit tougher, but it seems that versioned files referenced in CSS are not being updated.

For example, lets say you have the following files
public/test.png
public/test.css

.hello {
  width: 300px;
  height: 199px;
  background-image: url('test.png');
}

public/index.html

<html>
  <head>
    <link href="test.css" media="all" rel="stylesheet" type="text/css" />  
  </head>
  <body>
    <div class="hello"></div>
  </body>
</html>

If you feed it the following options, the line background-image: url('test.png'); does not get updated to the versioned file.

    {
      assets: [ 'test.css', 'test.js', 'test.png' ],
      grepFiles: [ 'index.html', 'test.css' ]
    }
techjacker commented 11 years ago

I'd never thought of replacing the assets inside the stylesheets, I did it with just html in mind, but it would be a good feature.

If the paths are all the same in the CSS then it should be doable. I'll have a tinker and will let you know how I get on.

techjacker commented 11 years ago

I've finally had a chance to check this out.

I added a couple of tests for the scenario you described, expecting them to fail but they passed! So then I did a manual test of the same scenario and it still worked. I experienced the weird feeling of being disappointed that something didn't fail.

Could you give any more details to help me recreate the problem? As you can see I'm struggling to find the bug.

techjacker commented 11 years ago

I added the manual test environment I created https://github.com/techjacker/node-version-assets/commit/302c97913f4568575873cbb038e45dce2fd6f0e9

See the /test-utils/manual-test-environment directory.

Maybe you could fork and recreate the scenario there?

shanestillwell commented 11 years ago

I have a git repo that you can easily use to test this.

git clone git@github.com:shanestillwell/grunt-version-assets.git
cd grunt-version-assets
npm install
grunt version_assets
cat public/test.ea04b12df55a945ab45039ae5e3a14f1.css

You'll notice that background-image: url('scouts.jpg'); is still pointing to the old version.

Here are the config options, that are found in Gruntfile.js

assets: ['*.css', '*.js', '*.png', '*.jpg'],
grepFiles: ['*.html', '*.css']

Thanks for your hard work

Shane A. Stillwell @shanestillwell 218.499.9205

On Monday, April 22, 2013 at 11:30 AM, techjacker wrote:

I added the manual test environment I created 302c979 (https://github.com/techjacker/node-version-assets/commit/302c97913f4568575873cbb038e45dce2fd6f0e9) See the /test-utils/manual-test-environment directory. Maybe you could fork and recreate the scenario there?

— Reply to this email directly or view it on GitHub (https://github.com/techjacker/node-version-assets/issues/7#issuecomment-16799354).

shanestillwell commented 11 years ago

Just a repost of the above comment (email doesn't do correct formatting)

I have a git repo that you can easily use to test this.

git clone git@github.com:shanestillwell/grunt-version-assets.git
cd grunt-version-assets
npm install
grunt version_assets
cat public/test.ea04b12df55a945ab45039ae5e3a14f1.css

You'll notice that background-image: url('scouts.jpg'); is still pointing to the old version.

Here are the config options, that are found in Gruntfile.js

assets: ['*.css', '*.js', '*.png', '*.jpg'],
grepFiles: ['*.html', '*.css']

Thanks for your hard work

techjacker commented 11 years ago

Thanks for uploading the repo. Just from glancing at the config options I'm pretty sure that the wildcards are the issue. I didn't build the library with those in mind, but again they would be a cool feature and I think it would be possible to enable them although it will take a bit of refactoring to get them working.

As a temporary fix I'd suggest naming each asset individually.

shanestillwell commented 11 years ago

Grunt takes care of expanding the wildcards. Here is what grunt expands them to

assets [ 'public/test.css', 'public/test.js', 'public/scouts.jpg' ] grepFiles [ 'public/index.html', 'public/test.css' ]

After you run it, you can see that asset names in index.html have been updated, but test.css has not.

Shane A. Stillwell @shanestillwell 218.499.9205

On Wednesday, April 24, 2013 at 4:36 AM, techjacker wrote:

Thanks for uploading the repo. Just from glancing at the config options I'm pretty sure that the wildcards are the issue. I didn't build the library with those in mind, but again they would be a cool feature and I think it would be possible to enable them although it will take a bit of refactoring to get them working. As a temporary fix I'd suggest naming each asset individually.

— Reply to this email directly or view it on GitHub (https://github.com/techjacker/node-version-assets/issues/7#issuecomment-16916809).

DzenisevichK commented 10 years ago

@shanestillwell As workaround I use 2 nested (new Version(...)).run invocations:

  1. For replacing images in css
assets: ['*.png', '*.jpg'],
grepFiles: ['*.css']
  1. For replacing css and js in html.
assets: ['*.css', '*.js'],
grepFiles: ['*.htm']