(refacto) various minor improvements

[vic1707]

In this PR I:

• simply moved the types in their own file and broke down every classes in their individual file
• changed a for loop to a forEach => reduces build size
• use ternary and arrowFunctions in token.ts => reduces build size
• changed a specific arrowFunction to a normal function => reduces build size for some reason

previous build size

Build "@owja/ioc" to dist:
        923 B: ioc.js.gz
        798 B: ioc.js.br
        922 B: ioc.mjs.gz
        806 B: ioc.mjs.br

new

Build "@owja/ioc" to dist:
        908 B: ioc.js.gz
        777 B: ioc.js.br
        911 B: ioc.mjs.gz
        801 B: ioc.mjs.br

Can you tell me which size is the most important ? Some modifications increase one of them while reducing another...

---

If you agree to enforce TypeSafety by removing the MaybeToken type and its logic it can be easily brought down to

Build "@owja/ioc" to dist:
        881 B: ioc.js.gz
        751 B: ioc.js.br
        888 B: ioc.mjs.gz
        783 B: ioc.mjs.br

[vic1707]

crap just discovered that types.ts isn't shipped in the dist folder

[hbroer]

hi, ty for this work as well. I will take a look into it too.

The MaybeToken exists because of backward compatibility. I thought about to declare the "old" symbol as deprecated and to be removed in 3.x But we can force the users to change the "service type" definitions from symboles to tokens. Maybe there is a 3rd way too. What to you think?

[vic1707]

The MaybeToken exists because of backward compatibility.

Makes total sense, I guess it would be too much of a change to fully abandon the classic Symbol in just one major release.

I wanted to remove it because what I look for in TypeScript is it's typesafety so having the token function was enough and removing MaybeToken lighten the package while making it more secure (same goes for the any type that I always forbid in my tsconfig.json).

Keeping it as deprecated in v2 and removing it in v3 is probably a good way of doing it. It's your library so the final choice is up to you 😉. I'm just a guy that loves your library and wants to help in developing it 😁.

[vic1707]

new current build size

Build "@owja/ioc" to dist:
        905 B: ioc.js.gz
        780 B: ioc.js.br
        908 B: ioc.mjs.gz
        804 B: ioc.mjs.br

can't tell why the .br are getting heavier while the .gz is getting smaller 🤔

[vic1707]

new build size with total explicit gains

Build "@owja/ioc" to dist:
        891 B: ioc.js.gz        (-32 B)
        780 B: ioc.js.br        (-18 B)
        890 B: ioc.mjs.gz       (-32 B)
        788 B: ioc.mjs.br       (-18 B)

[vic1707]

end of the simplification for Item and Container.get gives build sizes of

Build "@owja/ioc" to dist:
        863 B: ioc.js.gz        (-60 B => 6.50%)
        753 B: ioc.js.br        (-45 B => 5.64%)
        867 B: ioc.mjs.gz       (-55 B => 5,97%)
        752 B: ioc.mjs.br       (-54 B => 6,70%)

[vic1707]

I don't have more ideas for now, will do another PR to add more unit tests. Final build sizes

Build "@owja/ioc" to dist:
        861 B: ioc.js.gz        (-62 B => 6.72%)
        734 B: ioc.js.br        (-64 B => 8.02%)
        867 B: ioc.mjs.gz       (-55 B => 5.97%)
        742 B: ioc.mjs.br       (-64 B => 7.94%)

and updated gains with enforced typesafety with gains in B over the current modifications (percentage is the total gain) (this is for pure indication of what would be gained in a future major release 😉) Also discovered that enforcing typesafety allows to remove all never types and one unknown

Build "@owja/ioc" to dist:
        833 B: ioc.js.gz        (-28 B =>  9.75% in total)
        701 B: ioc.js.br        (-33 B => 12.16% in total)
        842 B: ioc.mjs.gz       (-25 B =>  8.68% in total)
        731 B: ioc.mjs.br       (-11 B =>  9.31% in total)

I'll be waiting your review and hopefully the merge in order to add typesafety on #57 (want to deal with conflicts now to have the least possible) 😉 P.S: sorry for all the notifications you got 😓

[vic1707]

not sure if enumerable: true is necessary in define.ts 🤔. If not it would give these results

Build "@owja/ioc" to dist:
        857 B: ioc.js.gz        (- 4 B)
        738 B: ioc.js.br        (+ 4 B)
        863 B: ioc.mjs.gz       (- 4 B)
        731 B: ioc.mjs.br       (-11 B)

and these for typesafety

Build "@owja/ioc" to dist:
        828 B: ioc.js.gz        (-5 B)
        699 B: ioc.js.br        (-2 B)
        837 B: ioc.mjs.gz       (-5 B)
        730 B: ioc.mjs.br       (-1 B)

[vic1707]

May I ask if you have time for a review? @hbroer

[vic1707]

Hey,

good work. The Plugin Type can be reduced a little and the code styles need to be fixed:

npm run prettier:fix

Yeah that's mybad, I guess my vscode kept my own settings instead of using yours.

But for everything else it looks like it is still working fine. I need to add a new CI provider. The Project runs with travis but they changed the plan and now this project hast no CI anymore. But that's another story.

I was thinking of adding Github Actions via a PR and wait for your review, I rarely use an external CI service and rely mostly on github to run the tests, block the merge etc... 🤔

After the two fixes I will accept the merge request. And sorry that it took a while.

No problem, I also have to excuse myself for the notifications 😓 I figured too late that you were probably on holidays.

[vic1707]

Also about the enumerable: true mentionned here https://github.com/owja/ioc/pull/58#issuecomment-1189520440 what do you think ?

[hbroer]

But for everything else it looks like it is still working fine. I need to add a new CI provider. The Project runs with travis but they changed the plan and now this project hast no CI anymore. But that's another story.

I was thinking of adding Github Actions via a PR and wait for your review, I rarely use an external CI service and rely mostly on github to run the tests, block the merge etc... 🤔

I just have added circleci. The config is in master. I think you need to merge the master so it will with the pull request. But not sure, still in try and error phase. :-D

[hbroer]

enumerable

I am not sure 100%, but I think it is better that it is enumerable because the property then behaves like all other properties which are set direcly like {hello:"world"}

We also don't need to fight for every byte. ;-)

[vic1707]

Then everything should be ready to go 😉

[hbroer]

yay it works :-D