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

Micro optimizations 2 #543

Merged
merged 2 commits into from
Mar 24, 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
2 changes: 1 addition & 1 deletion apps/benchmark/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@
"@types/benchmark": "^2.1.5",
"cross-env": "^7.0.3",
"shx": "^0.3.4",
"typescript": "^5.4.2"
"typescript": "^5.4.3"
}
}
4 changes: 2 additions & 2 deletions apps/site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
"@docusaurus/module-type-aliases": "^3.1.1",
"@docusaurus/tsconfig": "^3.1.1",
"@svgr/webpack": "^8.1.0",
"@types/react": "^18.2.67",
"@types/react": "^18.2.69",
"@types/react-dom": "^18.2.22",
"@types/uuid": "^9.0.8",
"raw-loader": "^4.0.2",
"shx": "^0.3.4",
"typescript": "^5.4.2"
"typescript": "^5.4.3"
},
"browserslist": {
"production": [
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"apps/benchmark"
],
"dependencies": {
"mobx": "^6.12.0",
"mobx": "^6.12.1",
"yjs": "^13.6.14"
},
"devDependencies": {
Expand All @@ -36,11 +36,11 @@
"eslint-plugin-import": "^2.29.1",
"eslint-plugin-react": "^7.34.1",
"eslint-plugin-react-hooks": "^4.6.0",
"netlify-cli": "^17.19.6",
"netlify-cli": "^17.20.1",
"prettier": "^3.2.5",
"prettier-plugin-organize-imports": "^3.2.4",
"turbo": "^1.12.5",
"typescript": "^5.4.2"
"turbo": "^1.13.0",
"typescript": "^5.4.3"
},
"packageManager": "[email protected]"
}
4 changes: 2 additions & 2 deletions packages/lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@
"ts-jest": "^29.1.2",
"ts-node": "^10.9.2",
"typedoc": "^0.25.12",
"typescript": "^5.4.2",
"vite": "^5.2.2"
"typescript": "^5.4.3",
"vite": "^5.2.4"
},
"dependencies": {
"fast-deep-equal": "^3.1.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/src/modelShared/BaseModelShared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export type ModelCreationData<M extends ModelWithProps> = ModelPropsToTransforme
* @deprecated Use SnapshotInOf<Model> instead.
*/
export type ModelFromSnapshot<
M extends ModelWithProps & { [fromSnapshotOverrideTypeSymbol]: any }
M extends ModelWithProps & { [fromSnapshotOverrideTypeSymbol]: any },
> = IsNeverType<
M[typeof fromSnapshotOverrideTypeSymbol],
ModelPropsToSnapshotCreationData<ModelPropsOf<M>>,
Expand Down
3 changes: 1 addition & 2 deletions packages/lib/src/modelShared/modelDecorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
runAfterModelDecoratorSymbol,
} from "../modelShared/modelSymbols"
import {
addHiddenProp,
failure,
getMobxVersion,
logWarning,
Expand Down Expand Up @@ -76,7 +75,7 @@ const runAfterClassInitialization = (
}

// the object is ready
addHiddenProp(instance, modelInitializedSymbol, true, false)
instance[modelInitializedSymbol] = true

runLateInitializationFunctions(instance, runBeforeOnInitSymbol)

Expand Down
117 changes: 54 additions & 63 deletions packages/lib/src/modelShared/sharedInternalModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { AnyType } from "../types/schemas"
import { tProp } from "../types/tProp"
import type { LateTypeChecker } from "../types/TypeChecker"
import { typesUnchecked } from "../types/utility/typesUnchecked"
import { assertIsObject, failure, propNameToSetterName } from "../utils"
import { addHiddenProp, assertIsObject, failure, propNameToSetterName } from "../utils"
import { chainFns } from "../utils/chainFns"
import { ModelClass, modelInitializedSymbol } from "./BaseModelShared"
import { modelInitializersSymbol } from "./modelClassInitializer"
Expand All @@ -24,35 +24,41 @@ import { modelMetadataSymbol, modelUnwrappedClassSymbol } from "./modelSymbols"
import { AnyModelProp, getModelPropDefaultValue, ModelProps, noDefaultValue, prop } from "./prop"
import { assertIsClassOrDataModelClass } from "./utils"

function getModelInstanceDataField<M extends AnyModel | AnyDataModel>(
model: M,
function createGetModelInstanceDataField<M extends AnyModel | AnyDataModel>(
modelProp: AnyModelProp,
modelPropName: string
): any {
// no need to use get since these vars always get on the initial $
const value = model.$[modelPropName]
): (model: M) => unknown {
const transformFn = modelProp._transform?.transform

if (!modelProp._transform) {
return value
if (!transformFn) {
// no need to use get since these vars always get on the initial $
return (model) => model.$[modelPropName]
}

return modelProp._transform.transform(value, model, modelPropName, (newValue) => {
// use apply set instead to wrap it in an action
// set the $ object to set the original value directly
applySet(model.$, modelPropName, newValue)
}) as any
const transformValue = (model: M, value: unknown) =>
transformFn(value, model, modelPropName, (newValue) => {
// use apply set instead to wrap it in an action
// set the $ object to set the original value directly
applySet(model.$, modelPropName, newValue)
})

return (model) => {
// no need to use get since these vars always get on the initial $
const value = model.$[modelPropName]
return transformValue(model, value)
}
}

function setModelInstanceDataField<M extends AnyModel | AnyDataModel>(
model: M,
modelProp: AnyModelProp,
modelPropName: string,
value: any
value: unknown
): void {
// hack to only permit setting these values once fully constructed
// this is to ignore abstract properties being set by babel
// see https://github.com/xaviergonz/mobx-keystone/issues/18
if (!(modelInitializedSymbol in model)) {
if (!(model as any)[modelInitializedSymbol]) {
return
}

Expand Down Expand Up @@ -89,7 +95,7 @@ type ToSnapshotProcessorFn = (sn: any, instance: any) => any

export function sharedInternalModel<
TProps extends ModelProps,
TBaseModel extends AnyModel | AnyDataModel
TBaseModel extends AnyModel | AnyDataModel,
>({
modelProps,
baseModel,
Expand Down Expand Up @@ -198,9 +204,12 @@ export function sharedInternalModel<
modelClass,
} as ModelConstructorOptions & DataModelConstructorOptions)

// add prop, it is faster than having to go to the root of the prototype to not find it
addHiddenProp(baseModel, modelInitializedSymbol, false, true)

// make sure abstract classes do not override prototype props
if (!propsToDeleteFromBase) {
propsToDeleteFromBase = Object.keys(composedModelProps).filter(
propsToDeleteFromBase = Object.keys(modelProps).filter(
(p) => !basePropNames.has(p as any) && Object.hasOwn(baseModel, p)
)
}
Expand Down Expand Up @@ -234,58 +243,40 @@ export function sharedInternalModel<
ThisModel[modelMetadataSymbol] = metadata
}

const newPrototype = Object.create(base.prototype)

ThisModel.prototype = new Proxy(newPrototype, {
get(target, p, receiver) {
if (receiver === ThisModel.prototype) {
return target[p]
}

const modelProp = !basePropNames.has(p as any) && composedModelProps[p as string]
return modelProp
? getModelInstanceDataField(receiver, modelProp, p as string)
: Reflect.get(target, p, receiver)
},

set(target, p, v, receiver) {
if (receiver === ThisModel.prototype) {
target[p] = v
return true
}

const modelProp = !basePropNames.has(p as any) && composedModelProps[p as string]
if (modelProp) {
setModelInstanceDataField(receiver, modelProp, p as string, v)
return true
}
return Reflect.set(target, p, v, receiver)
},

has(target, p) {
const modelProp = !basePropNames.has(p as any) && composedModelProps[p as string]
return !!modelProp || Reflect.has(target, p)
},
})

newPrototype.constructor = ThisModel
ThisModel.prototype = Object.create(base.prototype)
ThisModel.prototype.constructor = ThisModel

// add setter actions to prototype
for (const [propName, propData] of Object.entries(modelProps)) {
if (propData._setter === true) {
const setterName = propNameToSetterName(propName)
if (!(basePropNames as Set<string>).has(propName)) {
const get = createGetModelInstanceDataField(propData, propName)

const newPropDescriptor: any = modelAction(newPrototype, setterName, {
value: function (this: any, value: any) {
this[propName] = value
Object.defineProperty(ThisModel.prototype, propName, {
get() {
return get(this)
},
set(value: any) {
setModelInstanceDataField(this, propData, propName, value)
},
writable: true,
enumerable: false,
configurable: true,
enumerable: true,
configurable: false,
})
}

// we use define property to avoid the base proxy
Object.defineProperty(newPrototype, setterName, newPropDescriptor)
if (propData._setter === true) {
const setterName = propNameToSetterName(propName)

if (!(basePropNames as Set<string>).has(setterName)) {
const newPropDescriptor: any = modelAction(ThisModel.prototype, setterName, {
value: function (this: any, value: any) {
this[propName] = value
},
writable: true,
enumerable: false,
configurable: false,
})

Object.defineProperty(ThisModel.prototype, setterName, newPropDescriptor)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/lib/src/types/TypeChecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export function lateTypeChecker(fn: () => TypeChecker, typeInfoGen: TypeInfoGen)

Object.defineProperty(ltc, "typeInfo", {
enumerable: true,
configurable: true,
configurable: false,
get() {
return cachedTypeInfoGen(ltc as any)
},
Expand Down
4 changes: 2 additions & 2 deletions packages/mobx-keystone-yjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@
"spec.ts": "^1.1.3",
"ts-jest": "^29.1.2",
"ts-node": "^10.9.2",
"typescript": "^5.4.2",
"vite": "^5.2.2"
"typescript": "^5.4.3",
"vite": "^5.2.4"
},
"dependencies": {
"tslib": "^2.6.2"
Expand Down
Loading