-
Notifications
You must be signed in to change notification settings - Fork 2
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
refactor: combine react and core packages into one #448
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
0f711d4
to
ed47a8d
Compare
ed47a8d
to
c7728a0
Compare
0ff8063
to
4ef9c39
Compare
4ef9c39
to
181084d
Compare
c575c2a
to
0364290
Compare
0364290
to
926a138
Compare
926a138
to
aff846e
Compare
aff846e
to
8ea5f14
Compare
8ea5f14
to
9087405
Compare
9087405
to
a75d14e
Compare
This change combines the core and react packages into one - so that one import is required to use the full suite of functionality. It also updates the build: dist/core is the core libraries only, which are used for the CDN upload. dist/chat is the entire library - both react and chat.
Adds the node engine version to package.json, which ensures we enforce the correct engine version.
This change allows the SDK to be tested and run against node 18.
864b57d
to
9b704b0
Compare
Do we need to update the package.json to show the new node engine? |
@@ -63,8 +56,21 @@ | |||
"url": "https://github.com/ably/ably-chat-js/issues" | |||
}, | |||
"homepage": "https://github.com/ably/ably-chat-js#readme", | |||
"engines": { | |||
"node": ">=18.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added this one as we didn't have it before - this should be ok as we are supporting Node 18+
Forcing this merge as new checks aren't in main yet |
Context
CHADR-083
Docs PR: ably/docs#2385
Description
This change combines the core and react packages into one - so that one import is required to use the full suite of functionality.
Before users would do:
Now it is:
The build is updated as follows:
Also included are:
Checklist
Testing Instructions (Optional)
Please give the demo app a good test run to ensure that everything works as before.