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

CDN Bundle Fixes #333

Merged
merged 2 commits into from
Sep 2, 2024
Merged

CDN Bundle Fixes #333

merged 2 commits into from
Sep 2, 2024

Conversation

AndyTWF
Copy link
Collaborator

@AndyTWF AndyTWF commented Sep 2, 2024

Context

We were having some issues with using ably-js and ably-chat-js from the CDN, as the former doesn't export an ESM module (and the later tries to import it).

Description

  • Update the Vite library names for Chat and React to be AblyChat and AblyChatReact respectively. This means that users can do const chatClient = new AblyChat.ChatClient() instead of const chatClient = new window['ably-chat-js'].ChatClient() in the browser.
  • Update the README instructions for using the CDN to point to the UMD variant of Chat, which does work with ably-js.
  • Update the React dependencies to make react-dom and react/jsx-runtime externals - requiring the consumer to provide them and avoiding conflicts.

Checklist

  • QA'd by the author.
  • Unit tests created (if applicable).
  • Integration tests created (if applicable).
  • Follow coding style guidelines found here.
  • TypeDoc updated (if applicable).
  • (Optional) Update documentation for new features.
  • Browser tests created (if applicable).
  • In repo demo app updated (if applicable).

Testing Instructions (Optional)

Minimal working example:

<script src="https://cdn.ably.com/lib/ably.min-2.js"></script>
<script src="./path-to-ably-chat-js-umd-variant"></script>
<script>
  const realtime = new Ably.Realtime({
          key: 'YOUR KEY HERE',
          clientId: 'client_' + client_id,
        });
  const chatClient = new AblyChat.ChatClient(realtime);
</script>

If the above doesn't error, you're good!

Copy link

github-actions bot commented Sep 2, 2024

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 93.98% (🎯 92%) 2311 / 2459
🟢 Statements 93.98% (🎯 92%) 2311 / 2459
🟢 Functions 95.67% (🎯 93%) 199 / 208
🟢 Branches 94.1% (🎯 93%) 623 / 662
File CoverageNo changed files found.
Generated in workflow #1383

@github-actions github-actions bot temporarily deployed to staging/pull/333/typedoc September 2, 2024 13:20 Inactive
AndyTWF added a commit to ably/docs that referenced this pull request Sep 2, 2024
Related to ably/ably-chat-js#333

Currently, only the UMD variant of Chat works with ably-js, so we need to point to that.

Also expands the example of how to use the CDN-served library.
<script src="https://unpkg.com/react@18/umd/react.production.min.js"></script>
<script src="https://cdn.ably.com/lib/ably-chat-react.umd.cjs-0.js"></script>
<script>
const chatClient = new AblyChat.ChatClient(realtime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include some example calling AblyChatReact here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In hindsight... I'm going to get rid of React on the CDN. It's very complicated for what is probably very little usage.

@github-actions github-actions bot temporarily deployed to staging/pull/333/typedoc September 2, 2024 13:34 Inactive
AndyTWF added a commit to ably/docs that referenced this pull request Sep 2, 2024
Related to ably/ably-chat-js#333

Currently, only the UMD variant of Chat works with ably-js, so we need to point to that.

Also expands the example of how to use the CDN-served library.
@splindsay-92 splindsay-92 self-requested a review September 2, 2024 13:36
Copy link
Contributor

@splindsay-92 splindsay-92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and it works, nicely done!

The library was not working on the CDN when using the ESM version. This is because ably-js does not export
an ESM variant, and thus the import statement from Chat (which marks ably-js as external during build) fails
to resolve the module.

This change fixes the issue by updating the README to point at the UMD version of Chat when using the CDN: which does
work.

We also update the library names in the Vite config, which allows you to use the Chat SDK like so `const chatClient = new AblyChat.ChatClient()` rather
than having to do `new window["ably-chat-js"].ChatClient()`.
Bundling the DOM or runtime into chat means that the consuming application could have multiple versions of React in play. This
could cause issues down the line, so mark these packages as external: requiring consuming applications to provide them.

This is common practice in other packages, e.g. react-router.
@AndyTWF AndyTWF merged commit 7a5d889 into main Sep 2, 2024
8 checks passed
@AndyTWF AndyTWF deleted the chat-cdn-library branch September 2, 2024 15:36
AndyTWF added a commit to ably/docs that referenced this pull request Sep 4, 2024
Related to ably/ably-chat-js#333

Currently, only the UMD variant of Chat works with ably-js, so we need to point to that.

Also expands the example of how to use the CDN-served library.
AndyTWF added a commit to ably/docs that referenced this pull request Sep 4, 2024
Related to ably/ably-chat-js#333

Currently, only the UMD variant of Chat works with ably-js, so we need to point to that.

Also expands the example of how to use the CDN-served library.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants