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

Unify NodeJS and browser implementation #672

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

EzraBrooks
Copy link
Contributor

@EzraBrooks EzraBrooks commented Jan 25, 2024

Public API Changes

  • Removal of raw TCP socket support and NodeJS-stream-based Topic API
  • Removal of "RosLibNode" and "RosLibBrowser" in favor of a singular "RosLib" entrypoint.

Description

Removing this Node-specific functionality that seems to be largely untouched in almost 10 years allows having one unified entry point into the codebase. This decreases the maintenance burden of the library and opens the door to swapping these transports out at runtime instead of at build-time; with ESM and dynamic imports, that is very doable - but would require a lot of rearchitecture to accommodate a Promise-based async API for any functions that would use a dynamic import (since that returns a Promise)

@EzraBrooks

This comment was marked as resolved.

@EzraBrooks EzraBrooks force-pushed the unify-nodejs-and-browser-implementation branch from 5b8174a to 10aadfd Compare January 25, 2024 23:38
@EzraBrooks EzraBrooks added the v2 Issues and PRs, which will be fixed in v2, as it is a breaking change label Jan 25, 2024
@EzraBrooks
Copy link
Contributor Author

EzraBrooks commented Jan 25, 2024

This is yet another thing that has become a hurdle in #665, since import statements are evaluated at script load time, unlike require (which is just a function call).

@EzraBrooks EzraBrooks marked this pull request as ready for review January 25, 2024 23:44
@EzraBrooks EzraBrooks merged commit 160c128 into develop Feb 5, 2024
6 checks passed
@MatthijsBurgh MatthijsBurgh deleted the unify-nodejs-and-browser-implementation branch February 6, 2024 07:10
@EzraBrooks EzraBrooks added this to the 2.0.0 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Issues and PRs, which will be fixed in v2, as it is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants