From fe27acbb9dbb11158e5bb3eb0c8fc37b369ded2f Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 10 Nov 2023 12:02:30 -0300 Subject: [PATCH 1/5] Remove static methods from Message and PresenceMessage typings These declarations are unreachable via the typings. The user needs to access these methods via the MessageStatic and PresenceMessageStatic interfaces. --- ably.d.ts | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/ably.d.ts b/ably.d.ts index c938527491..908ccd6153 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -2393,22 +2393,6 @@ declare namespace Types { * @internal */ constructor(); - /** - * A static factory method to create a `Message` object from a deserialized Message-like object encoded using Ably's wire protocol. - * - * @param JsonObject - A `Message`-like deserialized object. - * @param channelOptions - A {@link ChannelOptions} object. If you have an encrypted channel, use this to allow the library to decrypt the data. - * @returns A promise which will be fulfilled with a `Message` object. - */ - static fromEncoded: (JsonObject: any, channelOptions?: ChannelOptions) => Promise; - /** - * A static factory method to create an array of `Message` objects from an array of deserialized Message-like object encoded using Ably's wire protocol. - * - * @param JsonArray - An array of `Message`-like deserialized objects. - * @param channelOptions - A {@link ChannelOptions} object. If you have an encrypted channel, use this to allow the library to decrypt the data. - * @returns A promise which will be fulfilled with an array of {@link Message} objects. - */ - static fromEncodedArray: (JsonArray: any[], channelOptions?: ChannelOptions) => Promise; /** * The client ID of the publisher of this message. */ @@ -2475,20 +2459,6 @@ declare namespace Types { * @internal */ constructor(); - /** - * Decodes and decrypts a deserialized `PresenceMessage`-like object using the cipher in {@link ChannelOptions}. Any residual transforms that cannot be decoded or decrypted will be in the `encoding` property. Intended for users receiving messages from a source other than a REST or Realtime channel (for example a queue) to avoid having to parse the encoding string. - * - * @param JsonObject - The deserialized `PresenceMessage`-like object to decode and decrypt. - * @param channelOptions - A {@link ChannelOptions} object containing the cipher. - */ - static fromEncoded: (JsonObject: any, channelOptions?: ChannelOptions) => Promise; - /** - * Decodes and decrypts an array of deserialized `PresenceMessage`-like object using the cipher in {@link ChannelOptions}. Any residual transforms that cannot be decoded or decrypted will be in the `encoding` property. Intended for users receiving messages from a source other than a REST or Realtime channel (for example a queue) to avoid having to parse the encoding string. - * - * @param JsonArray - An array of deserialized `PresenceMessage`-like objects to decode and decrypt. - * @param channelOptions - A {@link ChannelOptions} object containing the cipher. - */ - static fromEncodedArray: (JsonArray: any[], channelOptions?: ChannelOptions) => Promise; /** * The type of {@link PresenceAction} the `PresenceMessage` is for. */ From c7fd90ca76b0687ab40e749d3bc746253787b877 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 10 Nov 2023 12:05:12 -0300 Subject: [PATCH 2/5] Remove Message and PresenceMessage constructors from typings These declarations are unreachable via the typings. --- ably.d.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ably.d.ts b/ably.d.ts index 908ccd6153..126779e264 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -2387,12 +2387,6 @@ declare namespace Types { * Contains an individual message that is sent to, or received from, Ably. */ class Message { - /** - * Constructor for internal use. - * - * @internal - */ - constructor(); /** * The client ID of the publisher of this message. */ @@ -2453,12 +2447,6 @@ declare namespace Types { * Contains an individual presence update sent to, or received from, Ably. */ class PresenceMessage { - /** - * Constructor for internal use. - * - * @internal - */ - constructor(); /** * The type of {@link PresenceAction} the `PresenceMessage` is for. */ From de5ddfa7d97951697e13ed19d3396aae1aadb742 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 10 Nov 2023 14:57:44 -0300 Subject: [PATCH 3/5] Switch to using package.json exports for entrypoints This allows users to import the module-based version by writing ``` import { BaseRealtime } from 'ably/modules' ``` instead of having to import 'ably/build/modules'. I followed the guidance in [1] and [2]. [1] https://nodejs.org/api/packages.html#exports [2] https://webpack.js.org/guides/package-exports/ --- package.json | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 828499c041..101cbcac45 100644 --- a/package.json +++ b/package.json @@ -7,14 +7,17 @@ "url": "https://github.com/ably/ably-js/issues", "email": "support@ably.com" }, - "main": "./build/ably-node.js", - "typings": "./ably.d.ts", - "react-native": { - "./build/ably-node.js": "./build/ably-reactnative.js" - }, - "browser": { - "./build/ably-node.js": "./build/ably.js" + "exports": { + ".": { + "node": "./build/ably-node.js", + "react-native": "./build/ably-reactnative.js", + "default": "./build/ably.js" + }, + "./modules": { + "default": "./build/modules/index.js" + } }, + "typings": "./ably.d.ts", "files": [ "build/**", "ably.d.ts", From be44206acae21b95bcc2eeab04912f607374834d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 10 Nov 2023 16:55:16 -0300 Subject: [PATCH 4/5] Test that we can import the Types namespace --- test/package/browser/template/src/index.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/package/browser/template/src/index.ts b/test/package/browser/template/src/index.ts index e360efcc34..865bc21e60 100644 --- a/test/package/browser/template/src/index.ts +++ b/test/package/browser/template/src/index.ts @@ -1,13 +1,18 @@ -import { Realtime } from 'ably'; +import { Realtime, Types } from 'ably'; import { createSandboxAblyAPIKey } from './sandbox'; +// This function exists to check that we can import the Types namespace and refer to its types. +async function attachChannel(channel: Types.RealtimeChannel) { + await channel.attach(); +} + globalThis.testAblyPackage = async function () { const key = await createSandboxAblyAPIKey(); const realtime = new Realtime({ key, environment: 'sandbox' }); const channel = realtime.channels.get('channel'); - await channel.attach(); + await attachChannel(channel); const receivedMessagePromise = new Promise((resolve) => { channel.subscribe(() => { From b8397159a372939136ae1f41c071c010578880ca Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 1 Nov 2023 11:09:16 -0300 Subject: [PATCH 5/5] Add typings for tree-shakable version of library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I wasn’t sure of the best way to approach the typings of the modules (`Rest` etc). They should be opaque to the user (they shouldn’t be interacting with them directly and we want be free to change their interface in the future). There is an open issue to add support for opaque types to TypeScript [1], and people have suggested various sorts of ways of approximating them, which revolve around the use of `unique symbol` declarations. However, I don’t fully understand these solutions and so thought it best not to include them in our public API. So, for now, let’s just use `unknown`, the same way as we do for `CipherParams.key`. Resolves #1442. [1] https://github.com/Microsoft/Typescript/issues/202 --- .eslintrc.js | 2 +- .github/workflows/check.yml | 2 +- ably.d.ts | 107 +++++++++--------- modules.d.ts | 45 ++++++++ package.json | 6 +- test/package/browser/template/README.md | 11 +- test/package/browser/template/package.json | 2 +- .../server/resources/index-default.html | 11 ++ .../server/resources/index-modules.html | 11 ++ .../template/server/resources/index.html | 21 ---- .../template/server/resources/runTest.js | 9 ++ .../package/browser/template/server/server.ts | 5 +- .../src/{index.ts => index-default.ts} | 0 .../browser/template/src/index-modules.ts | 34 ++++++ .../browser/template/test/package.test.ts | 31 +++-- 15 files changed, 202 insertions(+), 95 deletions(-) create mode 100644 modules.d.ts create mode 100644 test/package/browser/template/server/resources/index-default.html create mode 100644 test/package/browser/template/server/resources/index-modules.html delete mode 100644 test/package/browser/template/server/resources/index.html create mode 100644 test/package/browser/template/server/resources/runTest.js rename test/package/browser/template/src/{index.ts => index-default.ts} (100%) create mode 100644 test/package/browser/template/src/index-modules.ts diff --git a/.eslintrc.js b/.eslintrc.js index 7f2ba0089d..8704799ce8 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -44,7 +44,7 @@ module.exports = { }, }, { - files: 'ably.d.ts', + files: ['ably.d.ts', 'modules.d.ts'], extends: [ 'plugin:jsdoc/recommended', ], diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 675343c945..f131a149da 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -21,5 +21,5 @@ jobs: - run: npm ci - run: npm run lint - run: npm run format:check - - run: npx tsc --noEmit ably.d.ts build/ably-webworker.min.d.ts + - run: npx tsc --noEmit ably.d.ts modules.d.ts build/ably-webworker.min.d.ts - run: npm audit --production diff --git a/ably.d.ts b/ably.d.ts index 126779e264..fa58110328 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -1700,32 +1700,7 @@ declare namespace Types { /** * A client that offers a simple stateless API to interact directly with Ably's REST API. */ - class Rest { - /** - * Construct a client object using an Ably {@link Types.ClientOptions} object. - * - * @param options - A {@link Types.ClientOptions} object to configure the client connection to Ably. - */ - constructor(options: Types.ClientOptions); - /** - * Constructs a client object using an Ably API key or token string. - * - * @param keyOrToken - The Ably API key or token string used to validate the client. - */ - constructor(keyOrToken: string); - /** - * The cryptographic functions available in the library. - */ - static Crypto: Types.Crypto; - /** - * Static utilities related to messages. - */ - static Message: Types.MessageStatic; - /** - * Static utilities related to presence messages. - */ - static PresenceMessage: Types.PresenceMessageStatic; - + abstract class Rest { /** * An {@link Types.Auth} object. */ @@ -1799,31 +1774,7 @@ declare namespace Types { /** * A client that extends the functionality of {@link Rest} and provides additional realtime-specific features. */ - class Realtime { - /** - * Construct a client object using an Ably {@link Types.ClientOptions} object. - * - * @param options - A {@link Types.ClientOptions} object to configure the client connection to Ably. - */ - constructor(options: Types.ClientOptions); - /** - * Constructs a client object using an Ably API key or token string. - * - * @param keyOrToken - The Ably API key or token string used to validate the client. - */ - constructor(keyOrToken: string); - /** - * The cryptographic functions available in the library. - */ - static Crypto: Types.Crypto; - /** - * Static utilities related to messages. - */ - static Message: Types.MessageStatic; - /** - * Static utilities related to presence messages. - */ - static PresenceMessage: Types.PresenceMessageStatic; + abstract class Realtime { /** * A client ID, used for identifying this client when publishing messages or for presence purposes. The `clientId` can be any non-empty string, except it cannot contain a `*`. This option is primarily intended to be used in situations where the library is instantiated with a key. A `clientId` may also be implicit in a token used to instantiate the library; an error will be raised if a `clientId` specified here conflicts with the `clientId` implicit in the token. */ @@ -2849,12 +2800,62 @@ declare namespace Types { /** * A client that offers a simple stateless API to interact directly with Ably's REST API. */ -export declare class Rest extends Types.Rest {} +export declare class Rest extends Types.Rest { + /** + * Construct a client object using an Ably {@link Types.ClientOptions} object. + * + * @param options - A {@link Types.ClientOptions} object to configure the client connection to Ably. + */ + constructor(options: Types.ClientOptions); + /** + * Constructs a client object using an Ably API key or token string. + * + * @param keyOrToken - The Ably API key or token string used to validate the client. + */ + constructor(keyOrToken: string); + /** + * The cryptographic functions available in the library. + */ + static Crypto: Types.Crypto; + /** + * Static utilities related to messages. + */ + static Message: Types.MessageStatic; + /** + * Static utilities related to presence messages. + */ + static PresenceMessage: Types.PresenceMessageStatic; +} /** * A client that extends the functionality of {@link Rest} and provides additional realtime-specific features. */ -export declare class Realtime extends Types.Realtime {} +export declare class Realtime extends Types.Realtime { + /** + * Construct a client object using an Ably {@link Types.ClientOptions} object. + * + * @param options - A {@link Types.ClientOptions} object to configure the client connection to Ably. + */ + constructor(options: Types.ClientOptions); + /** + * Constructs a client object using an Ably API key or token string. + * + * @param keyOrToken - The Ably API key or token string used to validate the client. + */ + constructor(keyOrToken: string); + /** + * The cryptographic functions available in the library. + */ + static Crypto: Types.Crypto; + /** + * Static utilities related to messages. + */ + static Message: Types.MessageStatic; + /** + * Static utilities related to presence messages. + */ + static PresenceMessage: Types.PresenceMessageStatic; +} /** * A generic Ably error object that contains an Ably-specific status code, and a generic status code. Errors returned from the Ably server are compatible with the `ErrorInfo` structure and should result in errors that inherit from `ErrorInfo`. diff --git a/modules.d.ts b/modules.d.ts new file mode 100644 index 0000000000..45e48bd18f --- /dev/null +++ b/modules.d.ts @@ -0,0 +1,45 @@ +import { Types } from './ably'; + +export declare const generateRandomKey: Types.Crypto['generateRandomKey']; +export declare const getDefaultCryptoParams: Types.Crypto['getDefaultParams']; +export declare const decodeMessage: Types.MessageStatic['fromEncoded']; +export declare const decodeEncryptedMessage: Types.MessageStatic['fromEncoded']; +export declare const decodeMessages: Types.MessageStatic['fromEncodedArray']; +export declare const decodeEncryptedMessages: Types.MessageStatic['fromEncodedArray']; +export declare const decodePresenceMessage: Types.PresenceMessageStatic['fromEncoded']; +export declare const decodePresenceMessages: Types.PresenceMessageStatic['fromEncodedArray']; +export declare const constructPresenceMessage: Types.PresenceMessageStatic['fromValues']; + +export declare const Rest: unknown; +export declare const Crypto: unknown; +export declare const MsgPack: unknown; +export declare const RealtimePresence: unknown; +export declare const WebSocketTransport: unknown; +export declare const XHRPolling: unknown; +export declare const XHRStreaming: unknown; +export declare const XHRRequest: unknown; +export declare const FetchRequest: unknown; +export declare const MessageInteractions: unknown; + +export interface ModulesMap { + Rest?: typeof Rest; + Crypto?: typeof Crypto; + MsgPack?: typeof MsgPack; + RealtimePresence?: typeof RealtimePresence; + WebSocketTransport?: typeof WebSocketTransport; + XHRPolling?: typeof XHRPolling; + XHRStreaming?: typeof XHRStreaming; + XHRRequest?: typeof XHRRequest; + FetchRequest?: typeof FetchRequest; + MessageInteractions?: typeof MessageInteractions; +} + +export declare class BaseRest extends Types.Rest { + constructor(options: Types.ClientOptions, modules: ModulesMap); +} + +export declare class BaseRealtime extends Types.Realtime { + constructor(options: Types.ClientOptions, modules: ModulesMap); +} + +export { Types }; diff --git a/package.json b/package.json index 101cbcac45..c4054bf155 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "default": "./build/ably.js" }, "./modules": { + "types": "./modules.d.ts", "default": "./build/modules/index.js" } }, @@ -21,6 +22,7 @@ "files": [ "build/**", "ably.d.ts", + "modules.d.ts", "resources/**", "src/**", "react/**" @@ -126,8 +128,8 @@ "lint": "eslint .", "lint:fix": "eslint --fix .", "prepare": "npm run build", - "format": "prettier --write --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts webpack.config.js Gruntfile.js scripts/*.js docs/chrome-mv3.md", - "format:check": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts webpack.config.js Gruntfile.js scripts/*.js", + "format": "prettier --write --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.js docs/chrome-mv3.md", + "format:check": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.js", "sourcemap": "source-map-explorer build/ably.min.js", "sourcemap:noencryption": "source-map-explorer build/ably.noencryption.min.js", "modulereport": "node scripts/moduleReport.js", diff --git a/test/package/browser/template/README.md b/test/package/browser/template/README.md index 2773d0a826..2a576916ac 100644 --- a/test/package/browser/template/README.md +++ b/test/package/browser/template/README.md @@ -5,7 +5,10 @@ This directory is intended to be used for testing the following aspects of the a - that its exports are correctly configured and provide access to ably-js’s functionality - that its TypeScript typings are correctly configured and can be successfully used from a TypeScript-based app that imports the package -The file `src/index.ts` imports the ably-js package and exports a function which briefly exercises its functionality. +It contains two files, each of which import ably-js in different manners, and which export a function which briefly exercises its functionality: + +- `src/index-default.ts` imports the default ably-js package (`import { Realtime } from 'ably'`). +- `src/index-modules.ts` imports the tree-shakable ably-js package (`import { BaseRealtime, WebSocketTransport, FetchRequest } from 'ably/modules'`). ## Why is `ably` not in `package.json`? @@ -15,6 +18,8 @@ The `ably` dependency gets added when we run the repository’s `test:package` p This directory exposes three package scripts that are to be used for testing: -- `build`: Uses esbuild to create a bundle containing `src/index.ts` and ably-js. -- `test`: Using the bundle created by `build`, tests that the code that exercises ably-js’s functionality is working correctly in a browser. +- `build`: Uses esbuild to create: + 1. a bundle containing `src/index-default.ts` and ably-js; + 2. a bundle containing `src/index-modules.ts` and ably-js. +- `test`: Using the bundles created by `build`, tests that the code that exercises ably-js’s functionality is working correctly in a browser. - `typecheck`: Type-checks the code that imports ably-js. diff --git a/test/package/browser/template/package.json b/test/package/browser/template/package.json index 0b1e74ff9c..1c91a386eb 100644 --- a/test/package/browser/template/package.json +++ b/test/package/browser/template/package.json @@ -4,7 +4,7 @@ "description": "", "main": "index.js", "scripts": { - "build": "esbuild --bundle src/index.ts --outdir=dist", + "build": "esbuild --bundle src/index-default.ts --outdir=dist && esbuild --bundle src/index-modules.ts --outdir=dist", "typecheck": "tsc -noEmit", "test-support:server": "ts-node server/server.ts", "test": "playwright test", diff --git a/test/package/browser/template/server/resources/index-default.html b/test/package/browser/template/server/resources/index-default.html new file mode 100644 index 0000000000..f71012bf99 --- /dev/null +++ b/test/package/browser/template/server/resources/index-default.html @@ -0,0 +1,11 @@ + + + + + Ably NPM package test (default export) + + + + + + diff --git a/test/package/browser/template/server/resources/index-modules.html b/test/package/browser/template/server/resources/index-modules.html new file mode 100644 index 0000000000..0135008065 --- /dev/null +++ b/test/package/browser/template/server/resources/index-modules.html @@ -0,0 +1,11 @@ + + + + + Ably NPM package test (tree-shakable export) + + + + + + diff --git a/test/package/browser/template/server/resources/index.html b/test/package/browser/template/server/resources/index.html deleted file mode 100644 index 0e5fbaa1d1..0000000000 --- a/test/package/browser/template/server/resources/index.html +++ /dev/null @@ -1,21 +0,0 @@ - - - - - Ably NPM package test - - - - - - diff --git a/test/package/browser/template/server/resources/runTest.js b/test/package/browser/template/server/resources/runTest.js new file mode 100644 index 0000000000..ccdc2d18e7 --- /dev/null +++ b/test/package/browser/template/server/resources/runTest.js @@ -0,0 +1,9 @@ +(async () => { + try { + await testAblyPackage(); + onResult(null); + } catch (error) { + console.log('Caught error', error); + onResult(error); + } +})(); diff --git a/test/package/browser/template/server/server.ts b/test/package/browser/template/server/server.ts index c371589aa3..6eefe26aba 100644 --- a/test/package/browser/template/server/server.ts +++ b/test/package/browser/template/server/server.ts @@ -3,8 +3,11 @@ import path from 'node:path'; async function startWebServer(listenPort: number) { const server = express(); + server.get('/', (req, res) => res.send('OK')); server.use(express.static(path.join(__dirname, '/resources'))); - server.use('/index.js', express.static(path.join(__dirname, '..', 'dist', 'index.js'))); + for (const filename of ['index-default.js', 'index-modules.js']) { + server.use(`/${filename}`, express.static(path.join(__dirname, '..', 'dist', filename))); + } server.listen(listenPort); } diff --git a/test/package/browser/template/src/index.ts b/test/package/browser/template/src/index-default.ts similarity index 100% rename from test/package/browser/template/src/index.ts rename to test/package/browser/template/src/index-default.ts diff --git a/test/package/browser/template/src/index-modules.ts b/test/package/browser/template/src/index-modules.ts new file mode 100644 index 0000000000..07a7751e3d --- /dev/null +++ b/test/package/browser/template/src/index-modules.ts @@ -0,0 +1,34 @@ +import { BaseRealtime, Types, WebSocketTransport, FetchRequest, generateRandomKey } from 'ably/modules'; +import { createSandboxAblyAPIKey } from './sandbox'; + +// This function exists to check that we can import the Types namespace and refer to its types. +async function attachChannel(channel: Types.RealtimeChannel) { + await channel.attach(); +} + +// This function exists to check that one of the free-standing functions (arbitrarily chosen) can be imported and does something vaguely sensible. +async function checkStandaloneFunction() { + const generatedKey = await generateRandomKey(); + if (!(generatedKey instanceof ArrayBuffer)) { + throw new Error('Expected to get an ArrayBuffer from generateRandomKey'); + } +} + +globalThis.testAblyPackage = async function () { + const key = await createSandboxAblyAPIKey(); + + const realtime = new BaseRealtime({ key, environment: 'sandbox' }, { WebSocketTransport, FetchRequest }); + + const channel = realtime.channels.get('channel'); + await attachChannel(channel); + + const receivedMessagePromise = new Promise((resolve) => { + channel.subscribe(() => { + resolve(); + }); + }); + + await channel.publish('message', { foo: 'bar' }); + await receivedMessagePromise; + await checkStandaloneFunction(); +}; diff --git a/test/package/browser/template/test/package.test.ts b/test/package/browser/template/test/package.test.ts index 8b4915b5a3..bb8e35fe43 100644 --- a/test/package/browser/template/test/package.test.ts +++ b/test/package/browser/template/test/package.test.ts @@ -1,18 +1,25 @@ import { test, expect } from '@playwright/test'; test.describe('NPM package', () => { - test('can be imported and provides access to Ably functionality', async ({ page }) => { - const pageResultPromise = new Promise((resolve, reject) => { - page.exposeFunction('onResult', (error: Error | null) => { - if (error) { - reject(error); - } else { - resolve(); - } + for (const scenario of [ + { name: 'default export', path: '/index-default.html' }, + { name: 'modular export', path: '/index-modules.html' }, + ]) { + test.describe(scenario.name, () => { + test('can be imported and provides access to Ably functionality', async ({ page }) => { + const pageResultPromise = new Promise((resolve, reject) => { + page.exposeFunction('onResult', (error: Error | null) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + + await page.goto(scenario.path); + await pageResultPromise; }); }); - - await page.goto('/'); - await pageResultPromise; - }); + } });