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

remove RxJS, use smaller Observables lib instead #197

Merged
merged 1 commit into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bin/cli.mts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type Range,
createClient,
} from '@openctx/client'
import { of } from 'rxjs'
import { Observable } from 'observable-fns'

function usageFatal(message: string): never {
console.error(message)
Expand Down Expand Up @@ -162,7 +162,7 @@ try {
}

const client = createClient({
configuration: () => of(config),
configuration: () => Observable.of(config),
providerBaseUri: import.meta.url,
logger: message => console.error('# ' + message),
makeRange: r => r,
Expand Down
6 changes: 4 additions & 2 deletions bin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@
"bin": {
"openctx": "dist/cli.mjs"
},
"files": ["dist/cli.mjs"],
"files": [
"dist/cli.mjs"
],
"scripts": {
"openctx": "pnpm run --silent bundle && node --no-warnings=ExperimentalWarning --experimental-network-imports dist/cli.mjs",
"bundle": "tsc --build && esbuild --log-level=error --platform=node --bundle --outdir=dist --format=esm --out-extension:.js=.mjs cli.mts",
"prepublishOnly": "tsc --build --clean && pnpm run -w --silent build && pnpm run --silent bundle"
},
"dependencies": {
"@openctx/client": "workspace:*",
"rxjs": "^7.8.1"
"observable-fns": "^0.6.1"
},
"devDependencies": {
"esbuild": "^0.21.3"
Expand Down
9 changes: 4 additions & 5 deletions client/browser/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "@openctx/browser-extension",
"version": "0.0.12",
"version": "0.0.13",
"license": "Apache-2.0",
"scripts": {
"dev": "vite",
Expand All @@ -17,16 +17,15 @@
"@openctx/provider": "workspace:*",
"@openctx/provider-hello-world": "workspace:*",
"@openctx/provider-links": "workspace:*",
"@openctx/provider-storybook": "workspace:*",
"@openctx/provider-prometheus": "workspace:*",
"@openctx/provider-storybook": "workspace:*",
"@openctx/ui-standalone": "workspace:*",
"clsx": "^2.0.0",
"deep-equal": "^2.2.3",
"jsonc-parser": "^3.2.0",
"observable-hooks": "^4.2.3",
"observable-fns": "^0.6.1",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"rxjs": "^7.8.1"
"react-dom": "^18.2.0"
},
"devDependencies": {
"@crxjs/vite-plugin": "^1.0.14",
Expand Down
23 changes: 17 additions & 6 deletions client/browser/src/background/background.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import '../shared/polyfills'
// ^^ import polyfills first

import { createClient } from '@openctx/client'
import type { UnsubscribableLike } from '@openctx/client/observable'
import type { Provider } from '@openctx/provider'
import helloWorldProvider from '@openctx/provider-hello-world'
import linksProvider from '@openctx/provider-links'
import prometheusProvider from '@openctx/provider-prometheus'
import storybookProvider from '@openctx/provider-storybook'
import { Subscription } from 'rxjs'
import { unsubscribe } from 'observable-fns'
import { addMessageListenersForBackgroundApi } from '../browser-extension/web-extension-api/rpc.js'
import { configurationChanges } from '../configuration.js'

Expand All @@ -31,23 +32,33 @@ function getBuiltinProvider(uri: string): Provider {
}

function main(): void {
const subscriptions = new Subscription()
const subscriptions: UnsubscribableLike[] = []

const client = createClient({
configuration: () => configurationChanges,
logger: console.error,
makeRange: r => r,
importProvider: uri => Promise.resolve({ default: getBuiltinProvider(uri) }),
})
subscriptions.add(() => client.dispose())
subscriptions.push(() => client.dispose())

subscriptions.add(
addMessageListenersForBackgroundApi({
subscriptions.push(
...addMessageListenersForBackgroundApi({
annotationsChanges: (...args) => client.annotationsChanges(...args),
}),
)

self.addEventListener('unload', () => subscriptions.unsubscribe(), { once: true })
self.addEventListener(
'unload',
() => {
for (const s of subscriptions) {
if (s) {
unsubscribe(s)
}
}
},
{ once: true },
)
}

// Browsers log an unhandled Promise here automatically with a nice stack trace, so we don't need to
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Observable } from 'rxjs'
import { Observable } from 'observable-fns'

/**
* Returns an Observable for a WebExtension API event listener.
Expand All @@ -7,13 +7,12 @@ import { Observable } from 'rxjs'
export const fromBrowserEvent = <F extends (...args: any[]) => void>(
emitter: browser.CallbackEventEmitter<F>,
): Observable<Parameters<F>> =>
// Do not use fromEventPattern() because of https://github.com/ReactiveX/rxjs/issues/4736
new Observable(subscriber => {
const handler: any = (...args: any) => subscriber.next(args)
new Observable(observer => {
const handler: any = (...args: any) => observer.next(args)
try {
emitter.addListener(handler)
} catch (error) {
subscriber.error(error)
observer.error(error)
return undefined
}
return () => emitter.removeListener(handler)
Expand Down
19 changes: 12 additions & 7 deletions client/browser/src/browser-extension/web-extension-api/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Observable, Subscription, type Unsubscribable } from 'rxjs'
import type { Unsubscribable } from '@openctx/client/observable'
import { Observable } from 'observable-fns'
import { isBackground } from '../../shared/env.js'
import type { BackgroundApi } from './types.js'

Expand Down Expand Up @@ -99,12 +100,16 @@ export function proxyBackgroundMethodReturningObservable<M extends keyof Backgro
/**
* Set up the background service worker to handle API requests from the content script.
*/
export function addMessageListenersForBackgroundApi(api: BackgroundApi): Unsubscribable {
export function addMessageListenersForBackgroundApi(api: BackgroundApi): Unsubscribable[] {
if (!isBackground) {
throw new Error('must be called from background')
}

const subscriptions = new Subscription()
const subscriptions: Unsubscribable[] = []
function removeSubscription(sub: Unsubscribable): void {
subscriptions.splice(subscriptions.indexOf(sub), 1)
sub.unsubscribe()
}

const handler = (
{ streamId, method, args }: RequestMessage,
Expand Down Expand Up @@ -135,25 +140,25 @@ export function addMessageListenersForBackgroundApi(api: BackgroundApi): Unsubsc
.sendMessage(senderTabId, { streamId, streamEvent: 'error', data: error.toString() })
.catch(console.error)
if (subscription) {
subscriptions.remove(subscription)
removeSubscription(subscription)
}
},
complete: () => {
browser.tabs
.sendMessage(senderTabId, { streamId, streamEvent: 'complete' })
.catch(console.error)
if (subscription) {
subscriptions.remove(subscription)
removeSubscription(subscription)
}
},
})
subscriptions.add(subscription)
subscriptions.push(subscription)
return Promise.resolve()
}

browser.runtime.onMessage.addListener(handler)

subscriptions.add(() => browser.runtime.onMessage.removeListener(handler))
subscriptions.push({ unsubscribe: () => browser.runtime.onMessage.removeListener(handler) })

return subscriptions
}
16 changes: 12 additions & 4 deletions client/browser/src/browser-extension/web-extension-api/storage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type Observable, concat, defer, filter, from, map, of } from 'rxjs'
import { concat, defer, filter, promiseToObservable } from '@openctx/client/observable'
import { Observable, map } from 'observable-fns'
import { platform } from '../../shared/platform.js'
import { fromBrowserEvent } from './fromBrowserEvent.js'
import type { LocalStorageItems, ManagedStorageItems, SyncStorageItems } from './types.js'
Expand Down Expand Up @@ -32,12 +33,12 @@ export const observeStorageKey = <
): Observable<ExtensionStorageItems[A][K] | undefined> => {
if (platform !== 'chrome-extension' && areaName === 'managed') {
// Accessing managed storage throws an error on Firefox and on Safari.
return of(undefined)
return Observable.of(undefined)
}
return concat(
// Start with current value of the item
defer(() =>
from(
promiseToObservable(
(storage[areaName] as browser.storage.StorageArea<ExtensionStorageItems[A]>).get(key),
).pipe(map(items => (items as ExtensionStorageItems[A])[key])),
),
Expand All @@ -52,7 +53,14 @@ export const observeStorageKey = <
[k in K]: browser.storage.StorageChange<ExtensionStorageItems[A][K]>
} => Object.prototype.hasOwnProperty.call(changes, key),
),
map(changes => changes[key].newValue),
map(
changes =>
(
changes as {
[k in K]: browser.storage.StorageChange<ExtensionStorageItems[A][K]>
}
)[key].newValue,
),
),
)
}
2 changes: 1 addition & 1 deletion client/browser/src/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { ClientConfiguration } from '@openctx/client'
import { type ParseError, parse as parseJSONC } from 'jsonc-parser'
import { type Observable, map } from 'rxjs'
import { type Observable, map } from 'observable-fns'
import { observeStorageKey } from './browser-extension/web-extension-api/storage.js'

const DEFAULT_CONFIG: ClientConfiguration = {
Expand Down
7 changes: 4 additions & 3 deletions client/browser/src/contentScript/contentScript.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import '../shared/polyfills'
import type { Annotation } from '@openctx/client'
import type { AnnotationsParams } from '@openctx/provider'
import deepEqual from 'deep-equal'
import { type Observable, combineLatest, distinctUntilChanged, mergeMap, throttleTime } from 'rxjs'
import { background } from '../browser-extension/web-extension-api/runtime.js'
import './contentScript.main.css'
import { combineLatest, distinctUntilChanged, mergeMap, throttleTime } from '@openctx/client/observable'
import type { Observable } from 'observable-fns'
import { debugTap } from './debug.js'
import { injectOnGitHubCodeView } from './github/codeView.js'
import { injectOnGitHubPullRequestFilesView } from './github/pullRequestFilesView.js'
Expand All @@ -26,13 +27,13 @@ const subscription = locationChanges
combineLatest(INJECTORS.map(injector => injector(location, annotationsChanges))),
),
)
.subscribe()
.subscribe({})
window.addEventListener('unload', () => subscription.unsubscribe())

function annotationsChanges(params: AnnotationsParams): Observable<Annotation[]> {
return background.annotationsChanges(params).pipe(
distinctUntilChanged((a, b) => deepEqual(a, b)),
throttleTime(200, undefined, { leading: true, trailing: true }),
throttleTime(200, { leading: true, trailing: true }),
debugTap(items => {
console.groupCollapsed('itemsChanges')
console.count('itemsChanges count')
Expand Down
11 changes: 8 additions & 3 deletions client/browser/src/contentScript/debug.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { tap } from 'rxjs'
import { tap } from '@openctx/client/observable'
import { Observable, type ObservableLike } from 'observable-fns'

/**
* Additional debug logging.
*/
export const DEBUG = true

/**
* Like RxJS's {@link tap}, but only run if {@link DEBUG} is true.
* Like the standard Observable {@link tap}, but only run if {@link DEBUG} is true.
*/
export const debugTap: typeof tap = DEBUG ? tap : () => source => source
export const debugTap: typeof tap = DEBUG
? tap
: () =>
(source: ObservableLike<any>): Observable<any> =>
Observable.from(source)
33 changes: 18 additions & 15 deletions client/browser/src/contentScript/detectElements.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Observable, map } from 'rxjs'
import { Observable, map } from 'observable-fns'

/**
* Return an Observable that emits the first DOM element that matches the given selector. If none is
Expand All @@ -17,21 +17,24 @@ export function withDOMElements<E extends Element>(selectors: string): Observabl
const els = document.querySelectorAll<E>(selectors)
if (els.length > 0) {
observer.next(Array.from(els))
} else {
let calls = 0
const MAX_CALLS = 10
const intervalHandle = setInterval(() => {
const els = document.querySelectorAll<E>(selectors)
if (els.length !== 0) {
observer.next(Array.from(els))
}
return undefined
}

let calls = 0
const MAX_CALLS = 10
const intervalHandle = setInterval(() => {
const els = document.querySelectorAll<E>(selectors)
if (els.length !== 0) {
observer.next(Array.from(els))
}

calls++
if (els.length > 0 || calls >= MAX_CALLS) {
clearInterval(intervalHandle)
}
}, 250)
observer.add(() => clearInterval(intervalHandle))
calls++
if (els.length > 0 || calls >= MAX_CALLS) {
clearInterval(intervalHandle)
}
}, 250)
return () => {
clearInterval(intervalHandle)
}
})
}
Loading
Loading