Consider reverting #110

[jderusse]

Versionning may be disabled at the user choice. Encore shouldn't ignore this choice to fix a technical naming issue.

For the final user, getting the bad file, may be better than no file at all (think about favicon.ico for instance). And IMO it shouldn't be a choice made be encore for you.

Please consider other alternatives like using the [path] when versioning is disabled or using a hash based on the filepath instead of the file content

[Lyrkan]

Hi @jderusse,

A bit of context about why we did that change: with the old behavior if you had two file with the same name in different subfolders (e.g. test1/test.png and test2/test.png) one would end up overwriting the other silently during builds (bug #73). The fix was to add the [hash] of the file to filenames.

Is there really a case where having a hash in the filename would cause an issue? Which problem would you have with a favicon for instance?

About considering alternatives:

• [path]: we talked about it in Allow to disable the default image and font loaders #103 and didn't see any benefit over just adding [hash]
• using a hash based on the filepath: I don't think you can do that using file-loader but if I'm wrong feel free to correct me

Note that for very specific cases you can also require your images using the file-loader?name=[name].[ext]!./image.png syntax.

[weaverryan]

Yo @jderusse!

I'm open to reverting if that's the right choice. But I'm also curious about your setup: are you referencing favicon.ico from somewhere in webpack (just to use that example)? Usually, if you're referencing an image or font from somewhere processed by Webpack (a CSS file or even a JS file), you don't care about the final path: Webpack rewrites your CSS to point to the new path, wherever it is. Even in JavaScript, if you reference an image, the new path to that image is returned:

// will be something like build/main.abc123.png
const imagePath = require('../images/main.png');

In other words, you shouldn't care about the final filename. And that's why we're curious about your setup :).

Cheers!

[jderusse]

Thank you for your feedback.

Actually, I don't care about the file path for a developer point of view. We both agree on that point :-)

But as soon as I expose a path to my users this path belong to the legacy and have to be maintained. You never known when this file will be requested by the user.
It could be few second between the time the user download the html page and the browser request the asset. Or several day if the image has been indexed by Google Image and a user click on it. Or several month if you include that image in an email.

You could answer that as long as the file doesn't change, the file path will remain the same and it'll work and so, if the path change, it means that the file is not the same and the user will retrieve an unexpected image. I would answer. Yes your right. But I prefer to provide the wrong asset than nothing (because most of the time the image with the same is really near of the previous one).

The only solution is to keep the assets forever. (In a CDN for instance) but it required to change a lot of thing in the infrastructure and the way we build the assets and served assets.

So my point is, if I choose to not use the versionning. Please, don't version files :-)

Configuring naming strategy (and EnableVersionning() as a shortcut) sound to be a good compromise.

Thank you for your time (I really like this Component). I would be happy to write PRs and help.

[Lyrkan]

I see your point but I don't totally agree with it.

A solution to your problem could be to keep old files between deployments. You don't need a CDN in order to do that: deploying the new build on top of the old one should do the job. That way, if an user tries to access to an old version of a file he'll retrieve it instead of the new one which could be entirely different and break your layout, email, etc.

In my opinion the old behavior ([name].ext) was a bigger and probably more common issue: we can't expect users to know that with the default settings (i.e. without enabling versioning) they can't use a given filename twice. Reverting #110 without modifying the old naming strategy would definitely not be a step forward. Wouldn't the suggestion in #125 (Encore.configureFilenames()) be enough?

[jderusse]

With automate build (like docker) in a CI, keeping build files between build is not a easy thing.

I don't says that my point is more important or common than the dupplicate name issue.
But I think that using the versionning was the simplest way to solve it, but wasn't the right solution.
In my opinion #110 broke the "versionning" option.

Indeed, #125 could be a workarround. And why not extending Enable/DisableVersionning ?

Encore.EnableVersionning = function() {
    Encore.configureFilenames({
        js: '[name].[hash:8].js',
        css: '[name].[hash:8].css',
        images: '[name].[hash:8].[ext]',
        fonts: '[name].[hash:8].[ext]'
    }
};
Encore.DisableVersionning = function() {
    Encore.configureFilenames({
        js: '[name].js',
        css: '[name].css',
        images: '[path]/[name].[ext]',
        fonts: '[path]/[name].[ext]'
    }
};

[Lyrkan]

Don't get me wrong I'm not saying that we made the right choice with this commit, I simply wanted to point out that if we do revert it we also need to do something else at the same time to prevent the old issue from coming back :)

With automate build (like docker) in a CI, keeping build files between build is not a easy thing.

True, but that also means that you'll have the same issue if you delete or move one of your assets (if we switch to [path][name].[ext]).

And why not extending Enable/DisableVersionning ?

One downside I see with this approach is that calling Encore.enableVersioning().configureFilenames({...}) would potentially not produce the same build than Encore.configureFilenames({...}).enableVersioning().
I really think that we should have two "default" configurations (one versioned and one non-versioned), choose which one to use based on if Encore.enableVersioning() has been called or not, and apply what has been passed to Encore.configureFilenames() on top of it.

Defaults configurations could be:

With versioning

• js: [name]-[chunkhash].js
• css: [name]-[contenthash].css
• images and fonts: [name]-[hash:8].[ext]

Without versioning

• js: [name].js
• css: [name].css
• images and fonts: [path][name].[ext]

[jderusse]

Sounds good to me :)

[elmariachi111]

yeah, this one is also rather important for us. Let me quickly describe the scenario. We're on Symfony2.8 and using the ?v= asset versioning strategy. One of our dependencies introduced a "loading.gif" that assetic once happily picked up and put it in our web/images directory. After migrating to encore (0.12) webpack copied that one into web/build/images/loading.gif and we kept on referencing that asset as {{ asset('images/loading.gif') }}. I guess the new (Symfony3.3) manifest asset versioning strategy is able to translate that one to loading.[hash:8].gif but our Twig helper only can do loading.gif?v=version, effectively breaking the asset on encore 0.13.

Long story short: we're desperately waiting for this pr! 😄

[weaverryan]

Thanks for sharing you use-case :).

I have just one question: are you manually requiring the loading.gif from some .js file just so that Webpack will move it into the build/ directory? I'm asking because the correct solution might be to use the copy-webpack-plugin... since this asset is not really handled by Webpack. But there could be more to your situation - that's why I'm asking :).

Cheers!

[elmariachi111]

It's actually worse :D In the bad old assetic times we had an assetic config that copied some image assets from (bower-) libraries to our web/images folder, leaving all names intact (so if we didn't watch and had assets with the same target name they would've overwritten each other).

after introducing encore/webpack we're requiring a library that comes with a loading.gif and references it somewhere in its assets. Webpack therefore picks it up and copies it to our image folder; When leaving the "enableVersioning" flag off, in 0.12 the image's name is simply kept ("loading.gif"), from 0.13 on it was rewritten. We're not requiring that file in our own js files but it's imported implicitly by that library.

We're actually using the copy-webpack-plugin to copy over most of our base assets into our build folder - that actually helped solving my situation for now: I just put a copy of that loading.gif in our source folder so it's copied (same behaviour as before) and I can simply continue to use the asset helper.

As said: if we were using Symfony 3.3 and its manifest based asset versioning scheme the asset helper should've been able to pick up the right file when consulting manifest.json. But not so on 2.8 ;)

[elmariachi111]

Hm. This seems to have been solved more or less In 0.14. But now I have a weird issue.

My settings:

.configureFilenames({
        js: '[name].js',
        images: 'images/[name].[ext]',
        fonts: 'fonts/[name].[ext]'
    })

When I encore dev filenames are e.g. build/fonts/icons.ttf. Manifest.json says, e.g:
"build/fonts/fontawesome-webfont.eot?v=4.7.0": "/build/fonts/fontawesome-webfont.eot"
✅

when I encore dev --watch and change some files the recompile will change filenames in manifest.json back to the old (0.13, aka hashed) filenames (.enableVersioning is off, but config.watchOptions = { ignored: /node_modules/ }) ;the effect takes ~1-2mins to appear) 🐛

"build/fonts/fontawesome-webfont.eot?v=4.7.0": "/build/fonts/fontawesome-webfont.674f50d2.eot"

[Lyrkan]

Indeed, that's really odd, filenames shouldn't change unless you call configureFilenames() again or reset() the conf.

I can't seem to reproduce the issue on my side though, would you mind sharing your webpack.config.js file and/or a minimal repository?

[elmariachi111]

Ah, guys, sorry, 🍺 on me: I had two encore watch procs running in the same dir at the same time (one survived a terminal kill and went on watching with the old config) 🙄

[Lyrkan]

No problem, glad you solved your issue :)

[andyexeter]

I agree that #110 should be reverted. I understand the problem that it solves, but it seems like a nasty, undocumented hack to me.

If I pass enableVersioning(false), versioning should be disabled on all files. After all, the method is not enableVersioningOnSomeFilesApartFromImagesBecauseOfABug().

[Lyrkan]

Hi @andyexeter,

Could you describe an use case in which having the hash included in images/fonts filenames would be an issue?

The naming strategy wasn't changed because of a bug, it was doing exactly what it was told to do before but that could lead to problems in very common cases:

• if we don't include the hash the filenames won't be modified but it'll prevent you from using the same names twice (subfolder1/logo.png, subfolder2/logo.png)
• if we use a hash it'll work out-of-the-box and most people won't care about it (since webpack will take care of generating the right paths)

In my opinion enableVersioning(false) is only useful for the files that you have to include manually in your html files if you are not using something like the html webpack plugin or the Asset component from Symfony (with the manifest-based strategy).

If you really want to remove the hash (or use something like [name].[ext]?[hash:8]) you can still do it using the Encore.configureFilenames({/* ... */}) method.

[andyexeter]

Hey @Lyrkan,

There are a few different use cases I can think of including SEO and link sharing but to be honest the use case is kind of irrelevant - the point I'm trying to make is that the hash being added to the images is undocumented, and unexpected when versioning has been explicitly disabled. I wasted a fair bit of time thinking there was something wrong with my config and was only able to find out the cause of this by reading through the project's issues.

Is it not a strong enough argument to say I just don't want hashes appended to my filenames? I don't care about cache-busting (even though this isn't being done to bust caches) - I want my pretty filenames (I'm speaking hypothetically here).

Just because something works, it doesn't mean it's the right thing to do. It'd still work if every single file processed by webpack was renamed 'doge1.js', 'wow_many_image.jpg', 'such_css.css' etc but that doesn't mean it should be done 😄

It just seems like putting a plaster/band-aid on a bug and forgetting about it. Instead, I believe we should be discussing why we can't have files with the same name in a different directory. If it's a problem with a loader or another of the project's dependencies I'd happily open an issue about it there. If I have dir1/file.jpg and dir2/file.jpg in my source assets, I want to see dir1/file.jpg and dir2/file.jpg in my build dir.

[Lyrkan]

but to be honest the use case is kind of irrelevant

I do think that use-cases are relevant since you shouldn't have to worry about filenames if there was no use-case requiring them to stay the same.

Is it not a strong enough argument to say I just don't want hashes appended to my filenames?

As I said in my previous reply you can now do that using configureFilenames.

It just seems like putting a plaster/band-aid on a bug and forgetting about it. Instead, I believe we should be discussing why we can't have files with the same name in a different directory.

The previous behavior was to use [name].[ext] for filenames if versioning was disabled:

• dir1/file.jpg would be emitted to <build_dir>/file.jpg
• dir2/file.jpg would also be emitted to <build_dir>/file.jpg

We talked about it in #103 and the only other viable alternative was to use [path][name].[ext] which could generate really ugly paths if you imported something outside of the context folder or from node_modules (which I'm convinced would have also been reported as an issue by other people).

[andyexeter]

As I said in my previous reply you can now do that using configureFilenames

This doesn't change the fact that there's no documentation saying enableVersioning(false) has no effect on images/fonts. How are people supposed to know this is the only way to remove hashes from images? The logical way to disable versioning on files is to pass false to enableVersioning.

Reading between the lines, I now understand the problem is because the directory structure isn't kept when moving files to the build directory, hence the potential for filename conflicts.

In my opinion, [path][name].[ext] is preferable to [name].[hash].ext - but I guess the problem is that this is completely subjective.

Thanks for the information. I'll use configureFilenames 😄

[weaverryan]

Let's add a note to the docs (and maybe also the jsdoc?) about the configureFilenames() method choosing to not version :)

[featuriz]

With the encore symfony4 becomes messy. So many extra files downloaded to project, creates bad impression. Using external tools(or manually) to compress and minify and to a single file is easy i think.
Something missing for me # #278

There is no simplicity

[stof]

@featuriz webpack-encore is meant to simplify the usage of webpack. If you just want to concatenate some files together and access external libraries through global variables (i.e. the old way of writing JS), you don't need webpack at all (and so you don't need encore). You could just use gulp with gulp-concat and gulp-uglify for instance (or just manual concatenation piped into the uglify CLI).

[weaverryan]

Closing - issue is old. Please open a new issue if needed! Thanks!