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

fix: improve resolver performance #623

Merged
merged 10 commits into from
May 16, 2024
5 changes: 5 additions & 0 deletions .changeset/fluffy-days-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@craftjs/core': minor
---

Resolve performance issue within resolveComponent.
73 changes: 50 additions & 23 deletions packages/core/src/utils/resolveComponent.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,65 @@
import { ERROR_NOT_IN_RESOLVER } from '@craftjs/utils';
import React from 'react';
import invariant from 'tiny-invariant';

import { Resolver } from '../interfaces';

type ReversedResolver = Map<React.ComponentType | string, string>;

type CachedResolverData = {
resolver: Resolver;
reversed: ReversedResolver;
};

let CACHED_RESOLVER_DATA: CachedResolverData | null = null;

const getReversedResolver = (resolver: Resolver): ReversedResolver => {
if (CACHED_RESOLVER_DATA && CACHED_RESOLVER_DATA.resolver === resolver) {
return CACHED_RESOLVER_DATA.reversed;
}

CACHED_RESOLVER_DATA = {
resolver,
reversed: new Map(),
};

for (const [name, comp] of Object.entries(resolver)) {
CACHED_RESOLVER_DATA.reversed.set(comp, name);
}

return CACHED_RESOLVER_DATA.reversed;
};

const getComponentName = (component: React.ElementType): string | undefined => {
return (component as any).name || (component as any).displayName;
};

const searchComponentInResolver = (
resolver: Resolver,
comp: React.ElementType
): string | null => {
const componentName = getComponentName(comp);
if (Object.hasOwn(resolver, componentName)) {
return componentName;
}

const name = getReversedResolver(resolver).get(comp);
return name !== undefined ? name : null;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can remove Line 41-44.

I think if the component doesn't exists in the reversed resolver, then we can assume that the component doesn't exist.

Even if the component's name/display name exist in the resolver, it might still belong to a different component, so this may lead to bugs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, I'll remove those lines now.

};

export const resolveComponent = (
resolver: Resolver,
comp: React.ElementType | string
) => {
const componentName = (comp as any).name || (comp as any).displayName;

const getNameInResolver = () => {
if (resolver[componentName]) {
return componentName;
}

for (let i = 0; i < Object.keys(resolver).length; i++) {
const name = Object.keys(resolver)[i];
const fn = resolver[name];

if (fn === comp) {
return name;
}
}

if (typeof comp === 'string') {
return comp;
}
};
): string => {
if (typeof comp === 'string') {
return comp;
}

const resolvedName = getNameInResolver();
const resolvedName = searchComponentInResolver(resolver, comp);

invariant(
resolvedName,
ERROR_NOT_IN_RESOLVER.replace('%node_type%', componentName)
ERROR_NOT_IN_RESOLVER.replace('%node_type%', getComponentName(comp))
);

return resolvedName;
Expand Down
Loading