Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Commit 3bc8eac

Browse files
sqssqs
committed
feat: remove ObservableEnvironment helper
BREAKING CHANGE: The ObservableEnvironment helper, which exposed common derived observables of the controller's environment, was removed. You need to create your own derived observables now using, e.g., `controller.environment.pipe(map({configuration}) => configuration), distinctUntilChanged())`.
1 parent ac790f2 commit 3bc8eac

File tree

7 files changed

+46
-90
lines changed

7 files changed

+46
-90
lines changed

‎src/client/controller.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { BehaviorSubject, Observable, Subject, Subscription, Unsubscribable } from 'rxjs'
2+
import { distinctUntilChanged, map } from 'rxjs/operators'
23
import { ContextValues } from 'sourcegraph'
34
import {
45
ConfigurationCascade,
@@ -20,7 +21,7 @@ import { ClientDocuments } from './api/documents'
2021
import { ClientLanguageFeatures } from './api/languageFeatures'
2122
import { ClientWindows } from './api/windows'
2223
import { applyContextUpdate, EMPTY_CONTEXT } from './context/context'
23-
import { createObservableEnvironment, EMPTY_ENVIRONMENT, Environment, ObservableEnvironment } from './environment'
24+
import { EMPTY_ENVIRONMENT, Environment } from './environment'
2425
import { Extension } from './extension'
2526
import { Registries } from './registries'
2627

@@ -75,6 +76,9 @@ export interface ControllerOptions<X extends Extension, C extends ConfigurationC
7576
export class Controller<X extends Extension, C extends ConfigurationCascade> implements Unsubscribable {
7677
private _environment = new BehaviorSubject<Environment<X, C>>(EMPTY_ENVIRONMENT)
7778

79+
/** The environment. */
80+
public readonly environment: Observable<Environment<X, C>> = this._environment
81+
7882
private _clientEntries = new BehaviorSubject<ExtensionConnection[]>([])
7983

8084
/** An observable that emits whenever the set of clients managed by this controller changes. */
@@ -190,7 +194,14 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
190194
subscription.add(connection)
191195
connection.listen()
192196
connection.onRequest('ping', () => 'pong')
193-
this.registerClientFeatures(connection, subscription, this.environment.configuration)
197+
this.registerClientFeatures(
198+
connection,
199+
subscription,
200+
this.environment.pipe(
201+
map(({ configuration }) => configuration),
202+
distinctUntilChanged()
203+
)
204+
)
194205
return connection
195206
}),
196207
}
@@ -222,7 +233,10 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
222233
subscription.add(
223234
new ClientWindows(
224235
client,
225-
this.environment.component,
236+
this.environment.pipe(
237+
map(({ component }) => component),
238+
distinctUntilChanged()
239+
),
226240
(params: ShowMessageParams) => this._showMessages.next({ ...params }),
227241
(params: ShowMessageRequestParams) =>
228242
new Promise<MessageActionItem | null>(resolve => {
@@ -235,7 +249,15 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
235249
)
236250
)
237251
subscription.add(new ClientCodeEditor(client, this.registries.textDocumentDecoration))
238-
subscription.add(new ClientDocuments(client, this.environment.textDocument))
252+
subscription.add(
253+
new ClientDocuments(
254+
client,
255+
this.environment.pipe(
256+
map(({ component }) => component && component.document),
257+
distinctUntilChanged()
258+
)
259+
)
260+
)
239261
subscription.add(
240262
new ClientLanguageFeatures(
241263
client,
@@ -249,8 +271,6 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
249271
subscription.add(new ClientCommands(client, this.registries.commands))
250272
}
251273

252-
public readonly environment: ObservableEnvironment<X, C> = createObservableEnvironment<X, C>(this._environment)
253-
254274
public set trace(value: Trace) {
255275
for (const client of this._clientEntries.value) {
256276
client.connection

‎src/client/environment.ts‎

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import { Observable, of } from 'rxjs'
2-
import { distinctUntilChanged, map } from 'rxjs/operators'
3-
import { TextDocument } from 'sourcegraph'
41
import { ConfigurationCascade } from '../protocol'
5-
import { isEqual } from '../util'
62
import { Context, EMPTY_CONTEXT } from './context/context'
73
import { Extension } from './extension'
84
import { TextDocumentItem } from './types/textDocument'
@@ -47,71 +43,3 @@ export interface Component {
4743
/** The document displayed by the component. */
4844
readonly document: TextDocumentItem
4945
}
50-
51-
/**
52-
* Observables for changes to the environment.
53-
*
54-
* Includes derived observables for convenience.
55-
*
56-
* @template X extension type, to support storing additional properties on extensions
57-
* @template C configuration cascade type
58-
*/
59-
export interface ObservableEnvironment<X extends Extension, C extends ConfigurationCascade> {
60-
/** The environment (and changes to it). */
61-
readonly environment: Observable<Environment<X, C>>
62-
63-
/** The environment's active component (and changes to it). */
64-
readonly component: Observable<Component | null>
65-
66-
/** The active component's text document (and changes to it). */
67-
readonly textDocument: Observable<Pick<TextDocument, 'uri' | 'languageId'> | null>
68-
69-
/** The environment's configuration cascade (and changes to it). */
70-
readonly configuration: Observable<C>
71-
72-
/** The environment's context (and changes to it). */
73-
readonly context: Observable<Context>
74-
}
75-
76-
/** An ObservableEnvironment that always represents the empty environment and never emits changes. */
77-
export const EMPTY_OBSERVABLE_ENVIRONMENT: ObservableEnvironment<any, any> = {
78-
environment: of(EMPTY_ENVIRONMENT),
79-
component: of(null),
80-
textDocument: of(null),
81-
configuration: of({}),
82-
context: of(EMPTY_CONTEXT),
83-
}
84-
85-
/**
86-
* Helper function for creating an ObservableEnvironment from the raw environment Observable.
87-
*
88-
* @template X extension type
89-
* @template C configuration cascade type
90-
*/
91-
export function createObservableEnvironment<X extends Extension, C extends ConfigurationCascade>(
92-
environment: Observable<Environment<X, C>>
93-
): ObservableEnvironment<X, C> {
94-
const component = environment.pipe(
95-
map(({ component }) => component),
96-
distinctUntilChanged((a, b) => isEqual(a, b))
97-
)
98-
const textDocument = component.pipe(
99-
map(component => (component ? component.document : null)),
100-
distinctUntilChanged((a, b) => isEqual(a, b))
101-
)
102-
const configuration = environment.pipe(
103-
map(({ configuration }) => configuration),
104-
distinctUntilChanged((a, b) => isEqual(a, b))
105-
)
106-
const context = environment.pipe(
107-
map(({ context }) => context),
108-
distinctUntilChanged((a, b) => isEqual(a, b))
109-
)
110-
return {
111-
environment,
112-
component,
113-
textDocument,
114-
configuration,
115-
context,
116-
}
117-
}

‎src/client/providers/contribution.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Subscription } from 'rxjs'
44
import { TestScheduler } from 'rxjs/testing'
55
import { ConfigurationCascade, ContributableMenu, Contributions } from '../../protocol'
66
import { EMPTY_COMPUTED_CONTEXT } from '../context/expr/evaluator'
7-
import { EMPTY_ENVIRONMENT, EMPTY_OBSERVABLE_ENVIRONMENT, Environment, ObservableEnvironment } from '../environment'
7+
import { EMPTY_ENVIRONMENT, Environment } from '../environment'
88
import { Extension } from '../extension'
99
import {
1010
contextFilter,
@@ -49,9 +49,9 @@ const FIXTURE_CONTRIBUTIONS_MERGED: Contributions = {
4949

5050
describe('ContributionRegistry', () => {
5151
function create(
52-
env: ObservableEnvironment<Extension, ConfigurationCascade> = EMPTY_OBSERVABLE_ENVIRONMENT
52+
env: Observable<Environment<Extension, ConfigurationCascade>> = of(EMPTY_ENVIRONMENT)
5353
): ContributionRegistry {
54-
return new ContributionRegistry(env.environment)
54+
return new ContributionRegistry(env)
5555
}
5656

5757
it('is initially empty', () => {
@@ -103,7 +103,7 @@ describe('ContributionRegistry', () => {
103103
): Observable<Contributions> {
104104
return super.getContributionsFromEntries(entries)
105105
}
106-
}(EMPTY_OBSERVABLE_ENVIRONMENT.environment)
106+
}(of(EMPTY_ENVIRONMENT))
107107
scheduler().run(({ cold, expectObservable }) =>
108108
expectObservable(
109109
registry.getContributionsFromEntries(
@@ -127,7 +127,7 @@ describe('ContributionRegistry', () => {
127127
): Observable<Contributions> {
128128
return super.getContributionsFromEntries(entries)
129129
}
130-
}(EMPTY_OBSERVABLE_ENVIRONMENT.environment)
130+
}(of(EMPTY_ENVIRONMENT))
131131
scheduler().run(({ cold, expectObservable }) =>
132132
expectObservable(
133133
registry.getContributionsFromEntries(

‎src/client/registries.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { EMPTY_OBSERVABLE_ENVIRONMENT } from './environment'
1+
import { of } from 'rxjs'
2+
import { EMPTY_ENVIRONMENT } from './environment'
23
import { Registries } from './registries'
34

45
describe('Registries', () => {
56
it('initializes empty registries', () => {
67
// tslint:disable-next-line:no-unused-expression
7-
new Registries(EMPTY_OBSERVABLE_ENVIRONMENT)
8+
new Registries(of(EMPTY_ENVIRONMENT))
89
})
910
})

‎src/client/registries.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { Observable } from 'rxjs'
12
import { ConfigurationCascade } from '../protocol'
2-
import { ObservableEnvironment } from './environment'
3+
import { Environment } from './environment'
34
import { Extension } from './extension'
45
import { CommandRegistry } from './providers/command'
56
import { ContributionRegistry } from './providers/contribution'
@@ -14,10 +15,10 @@ import { TextDocumentLocationProviderRegistry, TextDocumentReferencesProviderReg
1415
* @template C configuration cascade type
1516
*/
1617
export class Registries<X extends Extension, C extends ConfigurationCascade> {
17-
constructor(private environment: ObservableEnvironment<X, C>) {}
18+
constructor(private environment: Observable<Environment<X, C>>) {}
1819

1920
public readonly commands = new CommandRegistry()
20-
public readonly contribution = new ContributionRegistry(this.environment.environment)
21+
public readonly contribution = new ContributionRegistry(this.environment)
2122
public readonly textDocumentDefinition = new TextDocumentLocationProviderRegistry()
2223
public readonly textDocumentImplementation = new TextDocumentLocationProviderRegistry()
2324
public readonly textDocumentReferences = new TextDocumentReferencesProviderRegistry()

‎src/integration-test/context.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as assert from 'assert'
2+
import { distinctUntilChanged, map } from 'rxjs/operators'
23
import { ContextValues } from 'sourcegraph'
34
import { collectSubscribableValues, integrationTestContext } from './helpers.test'
45

@@ -7,7 +8,12 @@ describe('Context (integration)', () => {
78
it('updates context', async () => {
89
const { clientController, extensionHost, ready } = await integrationTestContext()
910
await ready
10-
const values = collectSubscribableValues(clientController.environment.context)
11+
const values = collectSubscribableValues(
12+
clientController.environment.pipe(
13+
map(({ context }) => context),
14+
distinctUntilChanged()
15+
)
16+
)
1117

1218
extensionHost.internal.updateContext({ a: 1 })
1319
await extensionHost.internal.sync()

‎src/integration-test/helpers.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export async function integrationTestContext(): Promise<
5858
// Confirm it is synchronous just in case, because a bug here would be hard to diagnose.
5959
let value!: Environment
6060
let sync = false
61-
clientController.environment.environment
61+
clientController.environment
6262
.pipe(first())
6363
.subscribe(environment => {
6464
value = environment

0 commit comments

Comments
 (0)