search

ctf0 / laravel-mix-versionhash

Auto append hash to file instead of using virtual one
MIT License
61 stars 18 forks source link

Update mix file removal #21

Closed Erutan409 closed 4 years ago

Erutan409 commented 4 years ago

Addresses #9

ctf0 commented 4 years ago

could u plz remove the semicolons

Erutan409 commented 4 years ago

@ctf0 Done.

ctf0 commented 4 years ago

awesome, thanx for ur hard work 🏅

Erutan409 commented 4 years ago

@ctf0 So, this is fun. I was doing a little more refactoring for the JSON manifest mutable stuff and realized what was part of the culprit from the beginning that negates the feature you just merged.

Using the build listener, I was able to remove the aforementioned logic and simplify the overall functionality.

ctf0 commented 4 years ago

awesome

Erutan409 commented 4 years ago

I'd guess more than likely no performance hit. That build listener triggers when webpack is finished compiling. So, it should be non-blocking and shouldn't impose any overhead.

ctf0 commented 4 years ago

perfect, last question, isnt mix.webpackConfig().then is the same as the event listener ?

i think in both u r waiting for webpack to finish

Erutan409 commented 4 years ago

The end result is the same, yes. But, the method chaining is a little redundant and, in my opinion, slightly misleading as to what's going on at first glance. Hooking onto webpackConfig() isn't doing anything in this context. Also, the way the manifest is being manipulated is not synchronous. Which, is probably why this was an issue with it not waiting until all the prerequisite logic was complete.

That last part is mostly a guess, though. I only started becoming familiar with the overall repo (yours and JeffreyWay's) within the last couple days. Otherwise, I would've suggested the last refactor, initially.