Skip to content

Commit

Permalink
cleanup some unnecessary mobx usage in legend-graph
Browse files Browse the repository at this point in the history
  • Loading branch information
akphi committed May 10, 2022
1 parent bd86b7b commit d0962ae
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 90 deletions.
8 changes: 4 additions & 4 deletions packages/legend-application/src/stores/LambdaEditorState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ export abstract class LambdaEditorState {
setCompilationError(compilationError: CompilationError | undefined): void {
// account for the lambda prefix offset in source information
if (compilationError?.sourceInformation) {
compilationError.setSourceInformation(
this.processSourceInformation(compilationError.sourceInformation),
compilationError.sourceInformation = this.processSourceInformation(
compilationError.sourceInformation,
);
}
this.compilationError = compilationError;
Expand All @@ -81,8 +81,8 @@ export abstract class LambdaEditorState {
setParserError(parserError: ParserError | undefined): void {
// account for the lambda prefix offset in source information
if (parserError?.sourceInformation) {
parserError.setSourceInformation(
this.processSourceInformation(parserError.sourceInformation),
parserError.sourceInformation = this.processSourceInformation(
parserError.sourceInformation,
);
}
this.parserError = parserError;
Expand Down
16 changes: 0 additions & 16 deletions packages/legend-graph/src/graphManager/action/EngineError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,10 @@
*/

import { ApplicationError } from '@finos/legend-shared';
import { observable, action, makeObservable } from 'mobx';
import type { SourceInformation } from '../action/SourceInformation';

export class EngineError extends ApplicationError {
sourceInformation?: SourceInformation | undefined;

constructor(message: string | undefined) {
super(message);

// TODO: check if we need this to be observable or not?
makeObservable(this, {
message: observable,
sourceInformation: observable,
setSourceInformation: action,
});
}

setSourceInformation(sourceInformation: SourceInformation | undefined): void {
this.sourceInformation = sourceInformation;
}
}

export class ParserError extends EngineError {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* limitations under the License.
*/

import { action, makeObservable, observable } from 'mobx';

const DEFAULT_TAB_SIZE = 2;

/**
Expand Down Expand Up @@ -53,19 +51,4 @@ export class TEMPORARY__AbstractEngineConfig {
setUseBase64ForAdhocConnectionDataUrls(val: boolean): void {
this.useBase64ForAdhocConnectionDataUrls = val;
}

constructor() {
makeObservable(this, {
env: observable,
currentUserId: observable,
baseUrl: observable,
useClientRequestPayloadCompression: observable,
useBase64ForAdhocConnectionDataUrls: observable,
setEnv: action,
setCurrentUserId: action,
setBaseUrl: action,
setUseClientRequestPayloadCompression: action,
setUseBase64ForAdhocConnectionDataUrls: action,
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (c) 2020-present, Goldman Sachs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { action, makeObservable, observable } from 'mobx';
import type { TEMPORARY__AbstractEngineConfig } from '../TEMPORARY__AbstractEngineConfig';
import { skipObserved } from './CoreObserverHelper';

export const observe_TEMPORARY__AbstractEngineConfig = skipObserved(
(
metamodel: TEMPORARY__AbstractEngineConfig,
): TEMPORARY__AbstractEngineConfig =>
makeObservable(metamodel, {
env: observable,
currentUserId: observable,
baseUrl: observable,
useClientRequestPayloadCompression: observable,
useBase64ForAdhocConnectionDataUrls: observable,
setEnv: action,
setCurrentUserId: action,
setBaseUrl: action,
setUseClientRequestPayloadCompression: action,
setUseBase64ForAdhocConnectionDataUrls: action,
}),
);
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,20 @@
* limitations under the License.
*/

import { makeObservable, observable, action } from 'mobx';
import { uuid } from '@finos/legend-shared';
import type { RelationalDatabaseConnection } from '../../../models/metamodels/pure/packageableElements/store/relational/connection/RelationalDatabaseConnection';

export class DatabasePattern {
uuid = uuid();
readonly uuid = uuid();
schemaPattern = '';
tablePattern = '';
escapeSchemaPattern?: boolean | undefined;
escapeTablePattern?: boolean | undefined;

constructor(schemaPattern: string, tablePattern: string) {
makeObservable(this, {
schemaPattern: observable,
tablePattern: observable,
escapeSchemaPattern: observable,
escapeTablePattern: observable,
setTablePattern: action,
setSchemaPattern: action,
});
this.schemaPattern = schemaPattern;
this.tablePattern = tablePattern;
}

setSchemaPattern(val: string): void {
this.schemaPattern = val;
}

setTablePattern(val: string): void {
this.tablePattern = val;
}
}

export class DatabaseBuilderConfig {
Expand All @@ -53,27 +36,13 @@ export class DatabaseBuilderConfig {
enrichPrimaryKeys = false;
enrichColumns = false;
patterns: DatabasePattern[] = [];

constructor() {
makeObservable(this, {
maxTables: observable,
enrichTables: observable,
enrichPrimaryKeys: observable,
enrichColumns: observable,
patterns: observable,
});
}
}

export class TargetDatabase {
name: string;
package: string;

constructor(_package: string, name: string) {
makeObservable(this, {
name: observable,
package: observable,
});
this.package = _package;
this.name = name;
}
Expand All @@ -85,11 +54,6 @@ export class DatabaseBuilderInput {
connection: RelationalDatabaseConnection;

constructor(connection: RelationalDatabaseConnection) {
makeObservable(this, {
targetDatabase: observable,
config: observable,
connection: observable,
});
this.connection = connection;
this.targetDatabase = new TargetDatabase('', '');
this.config = new DatabaseBuilderConfig();
Expand Down
2 changes: 2 additions & 0 deletions packages/legend-graph/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ export * from './graphManager/action/changeDetection/DSLExternalFormat_ObserverH
export * from './graphManager/action/changeDetection/DSLService_ObserverHelper';
export * from './graphManager/action/changeDetection/DSLGenerationSpecification_ObserverHelper';

export * from './graphManager/action/changeDetection/EngineObserverHelper';

// --------------------------------------------- TO BE MODULARIZED --------------------------------------------------

export * from './DSLMapping_Exports';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,12 @@ export abstract class PackageableElement implements Hashable, Stubable {
* Trigger recomputation on `hashCode` so if the element is observed, hash code computation will now
* remove itself from all observables it previously observed
*
* NOTE: we used to do this since we decorate `hashCode` with `computed({ keepAlive: true })` which
* poses a memory-leak threat
*
* See https://mobx.js.org/computeds.html#keepalive
*
* NOTE: for example, we used to do this since we decorate `hashCode` with
* `computed({ keepAlive: true })` which poses a memory-leak threat.
* However, since we're calling `keepAlive` actively now and dispose it right away to return `hashCode` to
* a normal computed value, we might not need this step anymore. But we're being extremely defensive here
* to avoid memory leak.
* See https://mobx.js.org/computeds.html#keepalive
*/
try {
this.hashCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { action, makeObservable, observable } from 'mobx';
import {
ContentType,
AbstractServerClient,
Expand Down Expand Up @@ -99,14 +98,6 @@ export class V1_EngineServerClient extends AbstractServerClient {
},
) {
super(config);

makeObservable(this, {
baseUrl: observable,
enableCompression: observable,
setBaseUrl: action,
setCompression: action,
});

this.queryBaseUrl = config.queryBaseUrl;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import { observer } from 'mobx-react-lite';
import { clsx, CheckSquareIcon, SquareIcon } from '@finos/legend-art';
import { isValidUrl } from '@finos/legend-shared';
import { useEditorStore } from '../EditorStoreProvider';
import { observe_TEMPORARY__AbstractEngineConfig } from '@finos/legend-graph';

export const DevTool = observer(() => {
const editorStore = useEditorStore();
// Engine
const engineConfig =
editorStore.graphManagerState.graphManager.TEMPORARY__getEngineConfig();
const engineConfig = observe_TEMPORARY__AbstractEngineConfig(
editorStore.graphManagerState.graphManager.TEMPORARY__getEngineConfig(),
);
const changeEngineClientBaseUrl: React.ChangeEventHandler<
HTMLInputElement
> = (event) =>
Expand Down

0 comments on commit d0962ae

Please sign in to comment.