search

nickjj / manifest-revision-webpack-plugin

Write out a manifest file containing your versioned webpack chunks and assets.
ISC License
124 stars 40 forks source link

Support DllPlugin output #18

Closed pauljz closed 6 years ago

pauljz commented 8 years ago

The webpack DllPlugin spits out an item with a name like dll vendors. This makes it through the initial checks and then crashes on the fs.lstatSync(item.name).isFile() check. This way we skip over it gracefully.

nickjj commented 8 years ago

Hmm, I wonder if there's a more elegant solution because hard coding for a specific plugin/string seems dirty. In theory other plugins could cause the same error but would require a custom blacklist string.

pauljz commented 8 years ago

Yeah, this doesn't seem super scalable. One alternative would be slapping a try/catch around the lstatSync call and skipping anything that doesn't exist.

nickjj commented 8 years ago

Yeah, that would work. As long as it printed out an informative error I think I don't mind the 2nd approach.

pauljz commented 8 years ago

Just pushed a new version that should be more flexible. This is basically the same as what you have except it continues on gracefully instead of crashing when it hits something like the dll lib artifact.

It's ugly code, but apparently try/catch is the idiomatic/only way to synchronously check for file existence in node. :(

nickjj commented 8 years ago

If we go down this route then I feel like it would be reasonable to print out a warning that details the error. Thoughts?

Silently ignoring it will result in a situation where some dude tears his hair out for 8 hours because the library didn't inform him of the problem.

pauljz commented 8 years ago

Yeah, you're right, that could be super frustrating. I worry about logging though because it could be a normal thing rather than an exception, in which case getting the output on every build would be undesirable too.

Maybe it's best to skip the file check completely. I tried that just now and it's successful. The dll plugin output gets caught by the assets check one line later anyway. If you had legit plugin output that wasn't a file (which would be really weird?) it would end up in the manifest file and you'd be able to find out about it that way pretty easily.

(In which case this PR just becomes removing line 63.)