Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Refactor runtimes #211

Open
Nyrox opened this issue Feb 9, 2023 · 2 comments
Open

Feature Request: Refactor runtimes #211

Nyrox opened this issue Feb 9, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@Nyrox
Copy link
Contributor

Nyrox commented Feb 9, 2023

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. :)

@Nyrox Nyrox added the enhancement New feature or request label Feb 9, 2023
@terehov
Copy link
Contributor

terehov commented Feb 9, 2023

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
Copy link
Contributor Author

Nyrox commented Feb 9, 2023

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. 😳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants