From 848fca2462028c3855927106364a8ffbabe6136a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 25 Jan 2021 16:39:20 -0500 Subject: [PATCH] Revive zen-observable-ts wrapper. Apollo Client currently uses the Observable implementation provided by the zen-observable npm package, since it is small and works well for our needs. Although zen-observable itself is not written in TypeScript, we use the companion package @types/zen-observable to provide types. We are actively considering switching from zen-observable to RxJS (#5749), though it remains to be seen how much of a breaking change that will be (and thus whether it makes sense for a minor or major version update). In the meantime, several issues (most recently #5961) have been opened pointing out that zen-observable effectively does not support importing its code as ECMAScript modules, even though it has a separate entry point (zen-observable/esm.js) that uses ESM syntax. This is a problem the zen-observable package could easily fix by adding a "module" field to its package.json, but @abdonrd tried to propose that change in a PR (as I requested), and has not heard back since June 2020: https://github.com/zenparsing/zen-observable/pull/74 Fortunately, Apollo maintains an npm package called zen-observable-ts, which was originally created to provide TypeScript types, before @types/zen-observable was introduced. This commit revives that wrapper package, so we can make sure both CommonJS and ESM exports are supported, with full TypeScript types, until the zen-observable maintainer gets around to merging that PR. I considered forking zen-observable, but I'm happy with this wrapping strategy, since reusing zen-observable makes it easier to take advantage of any future updates to the zen-observable package. I also considered using https://www.npmjs.com/package/patch-package to apply changes to node_modules/zen-observable/package.json upon installation, but that doesn't really work unless @apollo/client bundles the zen-observable implementation into itself, since otherwise the original (unpatched) zen-observable package will continue to be installed when developers npm install @apollo/client. The patch-package tool is great, but I don't think it's meant for libraries to use. Thankfully, this time around we do not need to hand-write the TypeScript types, since they can be re-exported from @types/zen-observable. I bumped the major version of zen-observable-ts, since the older versions (used by apollo-link) still get more than two million downloads per week. The source code for the zen-observable-ts package can now be found at https://github.com/apollographql/zen-observable-ts, rather than the old https://github.com/apollographql/apollo-link monorepo. --- package-lock.json | 27 ++++++++++++++++--------- package.json | 3 +-- src/utilities/observables/Observable.ts | 13 +++++++++--- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index ab7a42bb4f9..8f89f7a7716 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2750,9 +2750,9 @@ "dev": true }, "@types/zen-observable": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/@types/zen-observable/-/zen-observable-0.8.0.tgz", - "integrity": "sha512-te5lMAWii1uEJ4FwLjzdlbw3+n0FZNOvFXHxQDKeT0dilh7HOzdMzV2TrJVUzq8ep7J4Na8OUYPRLSQkJHAlrg==" + "version": "0.8.2", + "resolved": "https://registry.npmjs.org/@types/zen-observable/-/zen-observable-0.8.2.tgz", + "integrity": "sha512-HrCIVMLjE1MOozVoD86622S7aunluLb2PJdPfb3nYiEtohm8mIB/vyv0Fd37AdeMFrTUQXEunw78YloMA3Qilg==" }, "@ungap/global-this": { "version": "0.4.2", @@ -2902,6 +2902,16 @@ "requires": { "tslib": "^1.9.3" } + }, + "zen-observable-ts": { + "version": "0.8.21", + "resolved": "https://registry.npmjs.org/zen-observable-ts/-/zen-observable-ts-0.8.21.tgz", + "integrity": "sha512-Yj3yXweRc8LdRMrCC8nIc4kkjWecPAUVh0TI0OUrWXx6aX790vLcDlWca6I4vsyCGH3LpWxq0dJRcMOFoVqmeg==", + "dev": true, + "requires": { + "tslib": "^1.9.3", + "zen-observable": "^0.8.0" + } } } }, @@ -11512,13 +11522,12 @@ "integrity": "sha512-PQ2PC7R9rslx84ndNBZB/Dkv8V8fZEpk83RLgXtYd0fwUgEjseMn1Dgajh2x6S8QbZAFa9p2qVCEuYZNgve0dQ==" }, "zen-observable-ts": { - "version": "0.8.21", - "resolved": "https://registry.npmjs.org/zen-observable-ts/-/zen-observable-ts-0.8.21.tgz", - "integrity": "sha512-Yj3yXweRc8LdRMrCC8nIc4kkjWecPAUVh0TI0OUrWXx6aX790vLcDlWca6I4vsyCGH3LpWxq0dJRcMOFoVqmeg==", - "dev": true, + "version": "1.0.0-beta.4", + "resolved": "https://registry.npmjs.org/zen-observable-ts/-/zen-observable-ts-1.0.0-beta.4.tgz", + "integrity": "sha512-EnhcCIPbNSsAboYr6p9bVMFQ6naMK9Io7Qo8knQ9fFoMYUKgH9FHl+1oZYVV7x9N73LG4qU+kYyCf2Cs9MHqbw==", "requires": { - "tslib": "^1.9.3", - "zen-observable": "^0.8.0" + "@types/zen-observable": "^0.8.2", + "zen-observable": "^0.8.15" } } } diff --git a/package.json b/package.json index d6ef443c3db..02154ed399d 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ }, "dependencies": { "@graphql-typed-document-node/core": "^3.0.0", - "@types/zen-observable": "^0.8.0", "@wry/context": "^0.5.2", "@wry/equality": "^0.3.0", "@wry/trie": "^0.2.1", @@ -86,7 +85,7 @@ "symbol-observable": "^2.0.0", "ts-invariant": "^0.6.0", "tslib": "^1.10.0", - "zen-observable": "^0.8.14" + "zen-observable-ts": "^1.0.0-beta.4" }, "devDependencies": { "@babel/parser": "7.12.11", diff --git a/src/utilities/observables/Observable.ts b/src/utilities/observables/Observable.ts index fd721adcedd..3d10114c61c 100644 --- a/src/utilities/observables/Observable.ts +++ b/src/utilities/observables/Observable.ts @@ -1,11 +1,17 @@ -import Observable from 'zen-observable'; +import { + Observable, + Observer, + Subscription as ObservableSubscription, +} from 'zen-observable-ts'; // This simplified polyfill attempts to follow the ECMAScript Observable // proposal (https://github.com/zenparsing/es-observable) import 'symbol-observable'; -export type ObservableSubscription = ZenObservable.Subscription; -export type Observer = ZenObservable.Observer; +export type { + Observer, + ObservableSubscription, +}; // Use global module augmentation to add RxJS interop functionality. By // using this approach (instead of subclassing `Observable` and adding an @@ -18,4 +24,5 @@ declare global { } } (Observable.prototype as any)['@@observable'] = function () { return this; }; + export { Observable };