search

bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file
563 stars 71 forks source link

Drop compose-function #159

Closed TrySound closed 3 years ago

TrySound commented 3 years ago

compose-function does not add much value to the code. Unnecessary dependency can be avoided.

bholloway commented 3 years ago

@TrySound I guess this depends on whether you like tacit programming. Personally I really do but I appreciate lots of people don't.

Ah now I see that its only used in that one spot.

I'm not accepting PRs to master but interested on your thoughts on v4. ~But looks like the same file in v4 also has the compose. I'm happy if you want to raise a PR there.~ I have changed the PR target there but it seems you will need to rebase to resolve conflicts.

Unfortunately the test suite is annoying to run so I don't expect you to do that. So long as its an easy change to review I can just merge it and fix any problems later.

TrySound commented 3 years ago

I really like point-free style. Though compose function doesn't look the best part of it for me. I used it a couple of years ago and now can't remember what is the order of functions call. Pipeline operator would do a better job here. Anyway IMO such simple task does not worth a separate dependency in our bloated node_modules world.

TrySound commented 3 years ago

Btw in these cases much easier (again IMO) to call functions directly without wrapping as they are not even reused.

bholloway commented 3 years ago

Anyway IMO such simple task does not worth a separate dependency in our bloated node_modules world.

Agreed