search

onebeyond / systemic

📦 A minimal dependency injection framework.
https://onebeyond.github.io/systemic
MIT License
56 stars 7 forks source link

Feat/update syntax #41

Closed w3dani closed 3 years ago

w3dani commented 3 years ago

This PR updates the systemic syntax with arrow functions, const/let variable and also, I've removed lodash dependency

Tests are passing but I don't know if I could be missing something. Any feedback is welcome.

NOTE: There are still some function syntax because there is probably some kind of dependency with this

As soon as this PR is merged I'll keep working on some improvements I have in mind

MatteoDiPaolo commented 3 years ago

@w3dani I'm just curious:

cressie176 commented 3 years ago

Hi @w3dani, Sorry, but not keen on the const syntax for functions. Other things (such as the removal of lodash) I'm happy with. Can I suggest you create an issue for discussion prior to making sweeping changes please?

cressie176 commented 3 years ago

@w3dani I'm just curious:

  • why did you decide to remove lodash and define an utils for that?

I suspect it's because systemic imported the lodash libraries (lodash.get, lodash.set, etc) individually and this is now discouraged. Importing all of lodash is a pain because of the never ending number of npm audit warnings.

w3dani commented 3 years ago

Hi @w3dani, Sorry, but not keen on the const syntax for functions. Other things (such as the removal of lodash) I'm happy with. Can I suggest you create an issue for discussion prior to making sweeping changes please?

Sure! It's my first contribution and I wasn't sure about how to do it. I think the problem is I did a PR covering two different issues (syntax update and lodash removal). What do you think if I do another separate PR covering only the lodash removal and I create an issue with the syntax update and we discuss it?

@w3dani I'm just curious:

* why did you decide to remove lodash and define an utils for that?

* is using parentheses for one argument functions (e.g. `const func = (arg) => {}`) smth intended to be done or not?

As @cressie176 said, lodash is discouraged

w3dani commented 3 years ago

@cressie176 @MatteoDiPaolo Issue opened with the style upgrade: https://github.com/guidesmiths/systemic/issues/42