search

fullstack-build / tslog

📝 tslog - Universal Logger for TypeScript and JavaScript
https://tslog.js.org
MIT License
1.35k stars 63 forks source link

Feature Request: Refactor runtimes #211

Open Nyrox opened 1 year ago

Nyrox commented 1 year ago

Hey, first of all congrats on an amazing library, we really love it and want to use it everywhere we can. Currently I am looking at implementing a react-native runtime and it got me wondering in light of this open issue as well #203, if maybe it would be a good idea to refactor runtimes entirely, so that the library could work everywhere, potentially even with no runtime at all - if all you need is a JSON transport.

One way I can think of is to remove runtimes from tslog, export a 'Runtime' type and have the user pass it into the Logger constructor and then runtimes would be additional packages like tslog-runtime-browser, tslog-runtime-node etc.

Don't know if this over-engineered though. Let me know what you think. I am happy to do a lot of the implementation work either way. :)

terehov commented 1 year ago

That sounds interesting indeed. I have been trying to find a balance between 'batteries-included' and allowing everything to work without additional dependencies on one hand, and allowing customization of all steps on the other hand. Maybe it would be an option to implement a generic runtime that has no dependence on the environment?

Nyrox commented 1 year ago

Aight, I did a bit more talking with my tech lead and we ended up agreeing that it actually makes a lot of sense to go "batteries-included" and I was over engineering. I made a PR with an approach I think is pretty sane, which is to just DI the runtime into the BaseLogger and using the "browser" and "react-native" fields to have the users bundler choose the right entrypoint. It also allows anyone to just include the regular index.js file and write their own runtime.

React-Native btw. as we figured out just works out of the box with the browser runtime. So that is cool. Metro just doesn't understand the deep-replacement syntax in the package.json, hence why it needs a different entrypoint entirely.

Let me know what you think. 😳