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

Added typescript types to allow better integration #54

Closed
wants to merge 2 commits into from
Closed

Added typescript types to allow better integration #54

wants to merge 2 commits into from

Conversation

rgolea
Copy link

@rgolea rgolea commented Dec 30, 2022

  • Added typings to the package.
  • Update package.json

This should solve #25

* Added typings to the package.
* Update package.json
@rgolea rgolea marked this pull request as draft December 30, 2022 16:04
@josdejong
Copy link
Contributor

Thanks @rgolea , this looks good to me. Are there things that need to be addressed before merging this PR? (You've marked it as a Draft)

@rgolea
Copy link
Author

rgolea commented Jan 4, 2023

@josdejong yeah, for some reason, and I didn't have the time to look better into it (sorry for that), it loses the server typings. I will have it this week though. Also, do you plan on changing the project to compile to typescript? Would love to take a shot at it and also help remove some not needed polyfills.

@josdejong
Copy link
Contributor

Though I still maintain the library, I have no plans to put effort in timesync right now.

If you or someone else create a PR refactoring the library into TS I will be happy to review and merge it though, help would be very welcome.

@rgolea
Copy link
Author

rgolea commented Jan 23, 2023

I'm starting to refactor everything so I'm closing this PR at the moment. Please, @josdejong let me know if you agree to put it inside a monorepository. I want to use NX for that.

I want to fix the typescript issues but also use less polyfills (#30) and allow less instalation for the different packages. Are you okay with having something like: @timesync/server, @timesync/consumer and @timesync/peer (if needed)?

@rgolea rgolea closed this Jan 23, 2023
@josdejong
Copy link
Contributor

@rgolea I hadn't heard of NX before, but I'm ok with that if you think it's a good idea :). I'm curious to see how that will work out.

@rgolea
Copy link
Author

rgolea commented Jan 24, 2023

@josdejong it's basically Lerna on steroids. The problem I had was that I couldn't expose some internal typings.

I decided to go with this to have several packages and this way, we also prevent installing unnecessary parts.

I will finish this and I will show you the repo. If you like it, we can proceed with it, if not, we can scratch it off entirely.

@josdejong
Copy link
Contributor

👍 thanks!

@crossan007
Copy link

I took a different approach and refactored the whole source to TypeScript and configured tsc to emit type declarations https://github.com/crossan007/timesync

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

Successfully merging this pull request may close these issues.

3 participants