Skip to content

Commit

Permalink
fix: proxified model props were missing context before attribution (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ggazzo committed Mar 22, 2024
1 parent a3a807e commit 80d6c61
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/thirty-ducks-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/models": patch
---

Fix proxified model props were missing context before attribution
5 changes: 5 additions & 0 deletions .changeset/twenty-dolls-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fix error during migration 304. Throwing `Cannot read property 'finally' of undefined` error.
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"oauthapps",
"omnichannel",
"photoswipe",
"proxify",
"searchbox",
"tmid",
"tshow"
Expand Down
14 changes: 14 additions & 0 deletions packages/models/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export default {
preset: 'ts-jest',
errorOnDeprecated: true,
testEnvironment: 'jsdom',
modulePathIgnorePatterns: ['<rootDir>/dist/'],
testMatch: ['**/**.spec.ts'],
transform: {
'^.+\\.(t|j)sx?$': '@swc/jest',
},
moduleNameMapper: {
'\\.css$': 'identity-obj-proxy',
},
collectCoverage: true,
};
8 changes: 6 additions & 2 deletions packages/models/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
"eslint": "~8.45.0",
"jest": "~29.6.4",
"ts-jest": "~29.1.1",
"typescript": "~5.3.3"
"typescript": "~5.3.3",
"@swc/core": "^1.3.95",
"@swc/jest": "^0.2.29"
},
"dependencies": {
"@rocket.chat/model-typings": "workspace:^"
Expand All @@ -17,7 +19,9 @@
"lint:fix": "eslint --ext .js,.jsx,.ts,.tsx . --fix",
"test": "jest",
"dev": "tsc --watch --preserveWatchOutput -p tsconfig.json",
"build": "rm -rf dist && tsc -p tsconfig.json"
"build": "rm -rf dist && tsc -p tsconfig.json",
"unit": "jest",
"testunit": "jest"
},
"main": "./dist/index.js",
"typings": "./dist/index.d.ts",
Expand Down
91 changes: 91 additions & 0 deletions packages/models/src/proxify.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { proxify, registerModel } from './proxify';

type MockedModel = {
method: () => MockedModel;
};

describe('non lazy proxify', () => {
it('should keep this inside functions', () => {
const collectionMocked = proxify('collection') as MockedModel;
const collection = {
method() {
return this;
},
};
registerModel<any>('collection', collection);

expect(collectionMocked.method()).toBe(collection);
});
it('should throw an error if the model is not found', () => {
const collectionMocked = proxify('collection-not-found') as MockedModel;
expect(() => collectionMocked.method()).toThrowError('Model collection-not-found not found');
});

it('should return a proxified property', () => {
const collectionMocked = proxify('collection-prop') as {
prop: string;
};
const collection = {
prop: 'value',
};
registerModel<any>('collection-prop', collection);
expect(collectionMocked.prop).toBe('value');
});

it('should throw an error if trying to set a property from the proxified object', () => {
const collectionMocked = proxify('collection-prop') as {
prop: string;
};
const collection = {
prop: 'value',
};
registerModel<any>('collection-prop', collection);
expect(() => {
collectionMocked.prop = 'new value';
}).toThrowError('Models accessed via proxify are read-only, use the model instance directly to modify it.');
});
});

describe('lazy proxify', () => {
it('should keep this inside functions', () => {
const collectionMocked = proxify('collection-lazy') as MockedModel;
const collection = {
method() {
return this;
},
};

registerModel<any>('collection-lazy', () => collection);

expect(collectionMocked.method()).toBe(collection);
});

it('should throw an error if the model is not found', () => {
const collectionMocked = proxify('collection-not-found') as MockedModel;
expect(() => collectionMocked.method()).toThrowError('Model collection-not-found not found');
});

it('should return a proxified property', () => {
const collectionMocked = proxify('collection-prop') as {
prop: string;
};
const collection = {
prop: 'value',
};
registerModel<any>('collection-prop', () => collection);
expect(collectionMocked.prop).toBe('value');
});

it('should throw an error if trying to set a property from the proxified object', () => {
const collectionMocked = proxify('collection-prop') as {
prop: string;
};
const collection = {
prop: 'value',
};
registerModel<any>('collection-prop', () => collection);
expect(() => {
collectionMocked.prop = 'new value';
}).toThrowError('Models accessed via proxify are read-only, use the model instance directly to modify it.');
});
});
18 changes: 16 additions & 2 deletions packages/models/src/proxify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const models = new Map<string, IBaseModel<any>>();

function handler<T extends object>(namespace: string): ProxyHandler<T> {
return {
get: (_target: T, prop: keyof IBaseModel<any>): any => {
get: (_target: T, nameProp: keyof IBaseModel<any>): any => {
if (!models.has(namespace) && lazyModels.has(namespace)) {
const getModel = lazyModels.get(namespace);
if (getModel) {
Expand All @@ -19,7 +19,21 @@ function handler<T extends object>(namespace: string): ProxyHandler<T> {
throw new Error(`Model ${namespace} not found`);
}

return model[prop];
const prop = model[nameProp];

if (typeof prop === 'function') {
return prop.bind(model);
}

return prop;
},

set() {
if (process.env.NODE_ENV !== 'production') {
throw new Error('Models accessed via proxify are read-only, use the model instance directly to modify it.');
}
/* istanbul ignore next */
return true;
},
};
}
Expand Down

0 comments on commit 80d6c61

Please sign in to comment.