allow higher customization for compiling assets

[luceos]

One of the issues with the compiler logic is that it's hard to override it in case you need a different file naming convention. The current filenaming convention causes the forum to have no styling whenever an extension is enabled/disabled (cache is cleared) and you are using a CDN and/or horizontally scaled hosting environments. In case of cache invalidation nodes separately recompile the files, which isn't sensible in itself, but with a CDN involved becomes disastrous.

[askvortsov1]

The current filenaming convention causes the forum to have no styling whenever an extension is enabled/disabled (cache is cleared) and you are using a CDN and/or horizontally scaled hosting environments.

Would the alternative be keeping file names consistent across re-builds, and relying on the CDN to dictate which are cached when? And if so, wouldn't a better implementation of making this extensible be separating out the code that adds in that unique ID (I believe it's just a hash, but I don't remember off the top of my head) out?

[luceos]

Yes there are four potential files involved.

App CSS and js
Locale CSS and js

All of these are generated using a hash inside the filename, eg app-hash.js.

The common consensus is to use the hash inside a query param, app.js?v=hash for instance.

[askvortsov1]

Is there a reason why we wouldn't just want to adopt that as the default behavior?

With respect to the PR, I like the makeJsCompiler extraction, but I'm not as sure about populate and addSources. What could those be swapped out for?

[luceos]

I didn't want to change the behavior so much, I could modify the PR to adopt the new scheme, but I will do that in the coming days then.

[luceos]

Looking at online material again, there doesn't seem to be a specific consensus. I would keep the current logic but allow easier overriding, which this PR tries to solve.

[askvortsov1]

Really sorry for nitpicking on this one. One other question then: the current naming scheme (with the hash in filename) is determined by this>name, right? I'll need to take a look at the broader context again, but couldn't we extract the logic where this->name is created?

[luceos]

Yeah the Assets class is instantiated with a base name assigned to property name. This name is then used to generate all file names by prefixing these with the name:

• name-locale.css or name-locale.js
• name.css or name.js

The final filename, including the version hash, is later generated in each compiler using the abstract compiler getFilenameForRevision method. This is great and I can override this method by creating a new subclass for each compiler and overriding the method with a trait. But overriding the Assets class to use these new compilers isn't possible when these methods are private, as they are now.

Hey, this is not nitpicking. You need to ask these questions to get to the best result and I need to be patient in clarifying the exact need for these changes anyhow.

[askvortsov1]

Alright I think I get where you're coming from. Wouldn't the makeJsCompiler extraction into a protected be sufficient in this case though? Since that's the point where the compilers are inserted. I guess I don't understand the need for making populate and addSources

[luceos]

Okay, makes sense. Will revert those.