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

Refactor to avoid import cycle between inMemoryCache.ts and readFromStore.ts #8850

Merged

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Sep 27, 2021

The shouldCanonizeResults helper function introduced in #8822 was defined in inMemoryCache.ts but also used in readFromStore.ts. Since these two modules already mutually import TypeScript types from each other, the addition of a concrete/value export caused Rollup to begin displaying a warning about the new dependency cycle:

dist/cache/index.js → dist/cache/cache.cjs.js...
(!) Circular dependency
dist/cache/inmemory/inMemoryCache.js -> dist/cache/inmemory/readFromStore.js -> dist/cache/inmemory/inMemoryCache.js
created dist/cache/cache.cjs.js in 271ms

Dependency cycles in JavaScript are generally harmless and often unavoidable, but we can simplify the work of bundlers like Rollup by eliminating the cycles that are avoidable (as this one is).

Comment on lines 23 to 25
export {
InMemoryCache,
InMemoryCacheConfig,
} from './inmemory/inMemoryCache';
Copy link
Member Author

Choose a reason for hiding this comment

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

InMemoryCacheConfig is now exported by the

export * from './inmemory/types';

line below.

} from '../../utilities';

export const {
hasOwnProperty: hasOwn,
} = Object.prototype;

export function defaultDataIdFromObject(
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved defaultDataIdFromObject here to prevent another dependency cycle between helpers.ts and policies.ts.

return compact(defaultConfig, config);
}

export function shouldCanonizeResults(
Copy link
Member Author

Choose a reason for hiding this comment

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

The shouldCanonizeResults helper now lives in helpers.ts, whence it can be non-circularly imported by both inMemoryCache.ts and readFromStore.ts.

Copy link
Contributor

@brainkim brainkim left a comment

Choose a reason for hiding this comment

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

Circular dependencies. Not even once!

@benjamn benjamn merged commit d9a1039 into main Sep 27, 2021
@benjamn benjamn deleted the refactor-to-avoid-InMemoryCache-StoreWriter-import-cycle branch September 27, 2021 14:38
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants