-
Notifications
You must be signed in to change notification settings - Fork 60
1st trim #2025
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
1st trim #2025
Conversation
WalkthroughThe README.md file was extensively rewritten and condensed. The update replaces the original, detailed documentation with a much shorter version that provides a concise overview of Ably.js features, a simplified environments table, basic installation instructions, and a quick start link. Most detailed usage examples, advanced topics, and in-depth API documentation were removed. The troubleshooting section was reorganized and shortened, focusing on environment-specific issues. Contribution, support, and changelog sections were reduced to brief placeholders linking to external resources. No changes were made to any code or exported entities. Changes
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
README.md (3)
7-21
: Add missing trailing pipes to the Markdown table.The table rows lack trailing
|
, which violates the leading-and-trailing pipe style (MD055) and can break rendering in some Markdown parsers.Apply this diff to each row:
| Environment | Minimum Supported Version / Notes | -| **Chrome** | 58 (April 19, 2017) +| **Chrome** | 58 (April 19, 2017) | ... -| **Web Workers** | Supported in browser bundles and modular variant. +| **Web Workers** | Supported in browser bundles and modular variant. |🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
21-21: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe(MD055, table-pipe-style)
30-33
: Specify a language for the fenced code block.The NPM install snippet uses ``` without a language, triggering MD040. Adding a language (e.g.,
bash
) improves syntax highlighting.- ``` + ```bash npm install ably --save - ``` + ```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
30-30: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
49-51
: Fix grammar, spelling, and remove stray pipe in Quick Start.
- Insert a comma after "up and running".
- Correct "quickstart quide" → "quickstart guide".
- Remove the extraneous trailing
|
at the end of the line.- To quickly get up and running use the Ably Pub/Sub [quickstart quide](https://ably.com/docs/getting-started/quickstart). | + To quickly get up and running, use the Ably Pub/Sub [quickstart guide](https://ably.com/docs/getting-started/quickstart).🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: A comma might be missing here.
Context: ... ## Quick Start To quickly get up and running use the Ably Pub/Sub [quickstart quide]...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~50-~50: A comma might be missing here.
Context: ... ## Quick Start To quickly get up and running use the Ably Pub/Sub [quickstart quide]...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~113-~113: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ime()call to a separate file, such as
ably-client.js`, and export the client instance fro...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[style] ~113-~113: Consider shortening or rephrasing this to strengthen your wording.
Context: ...only be recreated when you specifically make changes to the ably-client.js
file, which should...
(MAKE_CHANGES)
[uncategorized] ~113-~113: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...en you specifically make changes to the ably-client.js
file, which should be far less freq...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
README.md
21-21: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
30-30: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 GitHub Actions: Lint
README.md
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-npm-package
- GitHub Check: test-node (16.x)
🔇 Additional comments (1)
README.md (1)
113-113
: Static analysis suggests hyphenation issues on adverb/adjective compounds, but the current phrasing ("far less frequent") is correct and no change is needed here.🧰 Tools
🪛 LanguageTool
[uncategorized] ~113-~113: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ime()call to a separate file, such as
ably-client.js`, and export the client instance fro...(HYPHENATED_LY_ADVERB_ADJECTIVE)
[style] ~113-~113: Consider shortening or rephrasing this to strengthen your wording.
Context: ...only be recreated when you specifically make changes to theably-client.js
file, which should...(MAKE_CHANGES)
[uncategorized] ~113-~113: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...en you specifically make changes to theably-client.js
file, which should be far less freq...(HYPHENATED_LY_ADVERB_ADJECTIVE)
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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
README.md (6)
1-127
: Prettier formatting neededThe linting pipeline indicates a Prettier check failure on this file. Please run:
prettier --write README.md
to automatically correct styling and formatting inconsistencies.
🧰 Tools
🪛 LanguageTool
[style] ~124-~124: This phrase is redundant. Consider using “outside”.
Context: ...-Renders. Move the client instantiation outside of your components to avoid recreating the...(OUTSIDE_OF)
[uncategorized] ~126-~126: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...call to a separate file, for example,
ably-client.js`) and export the client instance fro...(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
78-78: No empty links
null(MD042, no-empty-links)
🪛 GitHub Actions: Lint
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
5-11
: Consider adding REST API support in the features listAbly.js also provides REST API functionality. Including a bullet for REST APIs will give users a more complete overview of the SDK's capabilities.
Diff suggestion:
@@ -5,7 +5,8 @@ - [Pub/Sub messaging](https://ably.com/docs/realtime) - [Channels](https://ably.com/docs/channels) - [Presence](https://ably.com/docs/presence-occupancy/presence) - [Message history](https://ably.com/docs/storage-history/history) +- [REST APIs](https://ably.com/docs/rest) - [Delta compression](https://ably.com/docs/channels/options/deltas) - [Push notifications](https://ably.com/docs/push) - [Connection recovery](https://ably.com/docs/realtime/usage#connection-state-recovery)
48-52
: Add Yarn installation instructionsIncluding a Yarn example makes the installation guide more inclusive for users who prefer Yarn over npm.
Diff suggestion:
@@ 52-53 +### Yarn + +Install with Yarn: + +```bash +yarn add ably +```
116-116
: Add missing subject in conditionalThe sentence is missing the subject “you.” Update as follows:
- If receive an error during development such as "Connection Limit Exceeded", it may be due to: + If you receive an error during development such as "Connection Limit Exceeded", it may be due to:
124-124
: Remove unnecessary “of” in phrasingThe phrase “outside of your components” is wordy. Simplify it:
- Move the client instantiation outside of your components to avoid recreating the client on every re-render. + Move the client instantiation outside your components to avoid recreating the client on every re-render.🧰 Tools
🪛 LanguageTool
[style] ~124-~124: This phrase is redundant. Consider using “outside”.
Context: ...-Renders. Move the client instantiation outside of your components to avoid recreating the...(OUTSIDE_OF)
126-126
: Fix stray parenthesis in HMR exampleThere’s an extra
)
in the HMR instruction. It should read:- ... for example, `ably-client.js`) and export the client instance from there. + ... for example, `ably-client.js` and export the client instance from there.🧰 Tools
🪛 LanguageTool
[uncategorized] ~126-~126: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...call to a separate file, for example,
ably-client.js`) and export the client instance fro...(HYPHENATED_LY_ADVERB_ADJECTIVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
78-78: No empty links
null
(MD042, no-empty-links)
🪛 LanguageTool
README.md
[style] ~124-~124: This phrase is redundant. Consider using “outside”.
Context: ...-Renders. Move the client instantiation outside of your components to avoid recreating the...
(OUTSIDE_OF)
[uncategorized] ~126-~126: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...call to a separate file, for example,
ably-client.js`) and export the client instance fro...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 GitHub Actions: Lint
README.md
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-npm-package
- GitHub Check: test-node (16.x)
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
README.md (1)
128-131
: 🛠️ Refactor suggestion
⚠️ Potential issueFix HMR guidance code snippet: The constructor call is mis-formatted. Change:
- move the new `Ably.Realtime)` call into a separate file + move the `new Ably.Realtime(...)` call into a separate fileEnsure parentheses wrap the arguments correctly.
♻️ Duplicate comments (1)
README.md (1)
76-78
: Replace placeholder support link: The[support page](#)
link is still a placeholder. Update it to the actual support page (e.g.,https://ably.com/support
) so users can access help.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
78-78: No empty links
null(MD042, no-empty-links)
🧹 Nitpick comments (7)
README.md (7)
1-12
: Refine feature list capitalization and engagement: For consistency, capitalize each feature uniformly (e.g., "Message History" instead of "Message history", "Push Notifications" instead of "Push notifications"). Also, consider adding a brief example use case (e.g., chat, live dashboards) in the introduction to better illustrate Ably.js’s value.🧰 Tools
🪛 GitHub Actions: Lint
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
21-38
: Improve readability of the environments table: The "Node.js" entry combines version compatibility and bundling notes in one cell, which can be hard to scan. Consider splitting it into sub-bullets or separate "Supported Versions" / "Bundling Notes" columns for clarity.
43-57
: Expand installation options: Consider adding alternative package manager commands (e.g.,yarn add ably
,pnpm add ably
) and noting that only CommonJS (require('ably')
) is supported in the current ESM-less bundle. This will cater to more users.
60-65
: Use a version-agnostic CDN link: Rather than hardcodingably.min-2.js
, point to the latest stable release (e.g.,https://cdn.ably.com/lib/ably.min.js
), or instruct users to replacex.y.z
with their desired version. This helps ensure they pick up patches.
68-75
: Enhance Quick Start with an embedded snippet: Rather than only linking out, include a brief code snippet (e.g., initialize a client and publish a message) directly under the Quick Start heading. This gives users an immediate hands-on example.
80-97
: Clarify JSON snippet placeholders: The use of...
in the manifest.json example may confuse readers. Replace ellipses with a realistic key-value (e.g.,"name": "Your Extension",
) or add a note indicating that...
stands for other properties.
124-126
: Refine wording for clarity: Simplify by removing "of": change"outside of the component"
to"outside the component"
.
Example:- move the client instantiation outside of the component to prevent it from being recreated on every render. + move the client instantiation outside the component to prevent it from being recreated on every render.🧰 Tools
🪛 LanguageTool
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...(OUTSIDE_OF)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
78-78: No empty links
null
(MD042, no-empty-links)
🪛 LanguageTool
README.md
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...
(OUTSIDE_OF)
🪛 GitHub Actions: Lint
README.md
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-npm-package
- GitHub Check: test-browser (chromium)
- GitHub Check: test-node (16.x)
🔇 Additional comments (4)
README.md (4)
1-131
: Apply Prettier formatting: The lint pipeline reported Prettier formatting failures. Please runprettier --write README.md
to align the file with the project’s style guide.🧰 Tools
🪛 LanguageTool
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...(OUTSIDE_OF)
🪛 markdownlint-cli2 (0.17.2)
78-78: No empty links
null(MD042, no-empty-links)
🪛 GitHub Actions: Lint
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
15-17
: Changelog section is clear and concise. The link to CHANGELOG.md is correct. No changes needed.
99-110
: Next.js integration instructions are clear. The example correctly shows addingably
toserverComponentsExternalPackages
. No changes needed.
111-122
: SSR error handling guidance looks good. TheautoConnect
example correctly prevents connections during server-side rendering.
Trim number 1 Trim number 2 Trim number 3
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.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
README.md (2)
129-131
:⚠️ Potential issueFix mismatched parentheses in code example
The snippet showsAbly.Realtime)
with a stray closing parenthesis. It should readAbly.Realtime()
to correctly invoke the constructor.
1-131
:⚠️ Potential issuePrettier formatting errors detected
The GitHub Actions Prettier check failed. Please runprettier --write
(or integrate it into your commit hook) to bring the README into compliance with the repository’s formatting rules before merging.🧰 Tools
🪛 LanguageTool
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...(OUTSIDE_OF)
🪛 markdownlint-cli2 (0.17.2)
78-78: No empty links
null(MD042, no-empty-links)
🪛 GitHub Actions: Lint
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
♻️ Duplicate comments (1)
README.md (1)
78-78
: Replace placeholder support page link
The[support page](#)
link is still a placeholder and leads nowhere. Please update it to a real support URL (for example,https://ably.com/support
) so users can access technical help.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
78-78: No empty links
null(MD042, no-empty-links)
🧹 Nitpick comments (1)
README.md (1)
125-126
: Refine phrasing for conciseness
The phrase “outside of the component” is slightly wordy. Consider simplifying to “outside the component” for clearer, more concise documentation.🧰 Tools
🪛 LanguageTool
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...(OUTSIDE_OF)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
78-78: No empty links
null
(MD042, no-empty-links)
🪛 LanguageTool
README.md
[style] ~126-~126: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...
(OUTSIDE_OF)
🪛 GitHub Actions: Lint
README.md
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-npm-package
- GitHub Check: test-browser (chromium)
- GitHub Check: test-node (16.x)
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.
Actionable comments posted: 0
🧹 Nitpick comments (7)
README.md (7)
1-2
: Add repository badges for status and version
Consider adding standard badges (e.g., build status, npm version, coverage) at the top of the README to quickly convey project health and current release.
3-4
: Reduce repetitive phrasing
The product name is repeated in the second sentence. Consider using a pronoun for readability:- Ably Pub/Sub JavaScript SDK supports building realtime applications with features like: + It supports building realtime applications with features like:
17-32
: Ensure consistent punctuation in table cells
Some entries end with a period while others do not. For a polished look, either include or omit terminal punctuation uniformly across all table rows.
45-46
: Simplify npm install command
Since npm ≥5 adds dependencies topackage.json
by default, the--save
flag is no longer required:- npm install ably --save + npm install ably
50-52
: Use modern JS declaration
Preferconst
overvar
for importing modules in Node.js:- var Ably = require('ably'); + const Ably = require('ably');
118-120
: Refine wording per style guide
Replace “outside of the component” with “outside the component” to avoid redundant phrasing:- Instead, move the client instantiation outside of the component to prevent it from being recreated on every render. + Instead, move the client instantiation outside the component to prevent it from being recreated on every render.🧰 Tools
🪛 LanguageTool
[style] ~120-~120: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...(OUTSIDE_OF)
130-133
: Consider adding a License section
Most repositories include a## License
section linking to the LICENSE file to clarify usage terms.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~120-~120: This phrase is redundant. Consider using “outside”.
Context: ... Instead, move the client instantiation outside of the component to prevent it from being ...
(OUTSIDE_OF)
🔇 Additional comments (1)
README.md (1)
72-72
: Support link updated correctly
The support page now points to a real URL instead of a placeholder.
Summary by CodeRabbit