Skip to content

Commit

Permalink
Merge pull request #34 from /issues/33
Browse files Browse the repository at this point in the history
Warn about potentially non-unique cache keys
  • Loading branch information
bvaughn authored Jun 9, 2023
2 parents 801d298 + 69066fb commit 3b5999d
Show file tree
Hide file tree
Showing 20 changed files with 376 additions and 249 deletions.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"path-browserify": "^1.0.0",
"prettier": "latest",
"process": "^0.11.10",
"typescript": "^4.9.5"
"typescript": "^5.0.4"
},
"version": "0.0.0",
"workspaces": [
Expand All @@ -49,5 +49,8 @@
"___experimentalFlags_WILL_CHANGE_IN_PATCH": {
"importsConditions": true
}
},
"@parcel/resolver-default": {
"packageExports": true
}
}
2 changes: 1 addition & 1 deletion packages/array-sorting-utilities/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@types/jest": "^29.4.0",
"jest": "^29.4.3",
"ts-jest": "^29.0.5",
"typescript": "^4.9.5"
"typescript": "^5.0.4"
},
"browserslist": [
"Chrome 79"
Expand Down
2 changes: 1 addition & 1 deletion packages/interval-utilities/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@types/jest": "^29.4.0",
"jest": "^29.4.3",
"ts-jest": "^29.0.5",
"typescript": "^4.9.5"
"typescript": "^5.0.4"
},
"dependencies": {
"point-utilities": "^0.0.2"
Expand Down
2 changes: 1 addition & 1 deletion packages/point-utilities/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@types/jest": "^29.4.0",
"jest": "^29.4.3",
"ts-jest": "^29.0.5",
"typescript": "^4.9.5"
"typescript": "^5.0.4"
},
"browserslist": [
"Chrome 79"
Expand Down
3 changes: 3 additions & 0 deletions packages/suspense/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/** @type {import('ts-jest').JestConfigWithTsJest} */
module.exports = {
preset: "ts-jest",
testEnvironmentOptions: {
customExportConditions: ["development"],
},
testMatch: ["**/*.test.{ts,tsx}"],
};
13 changes: 12 additions & 1 deletion packages/suspense/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,23 @@
"import": "./dist/suspense.cjs.mjs",
"default": "./dist/suspense.cjs.js"
},
"development": {
"module": "./dist/suspense.development.esm.js",
"import": "./dist/suspense.development.cjs.mjs",
"default": "./dist/suspense.development.cjs.js"
},
"module": "./dist/suspense.esm.js",
"import": "./dist/suspense.cjs.mjs",
"default": "./dist/suspense.cjs.js"
},
"./package.json": "./package.json"
},
"imports": {
"#is-development": {
"development": "./src/env-conditions/development.ts",
"default": "./src/env-conditions/production.ts"
}
},
"types": "dist/suspense.cjs.d.ts",
"scripts": {
"build": "parcel build",
Expand All @@ -45,7 +56,7 @@
"react": "0.0.0-experimental-49f741046-20230305",
"react-dom": "0.0.0-experimental-49f741046-20230305",
"ts-jest": "^29.0.5",
"typescript": "^4.9.5"
"typescript": "^5.0.4"
},
"dependencies": {
"array-sorting-utilities": "^0.0.1",
Expand Down
22 changes: 22 additions & 0 deletions packages/suspense/src/cache/createCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,4 +731,26 @@ describe("createCache", () => {
});
});
});

describe("development warnings", () => {
it("should warn if a key contains a stringified object", async () => {
const cache = createCache<[Object, string], boolean>({
getKey: ([object, id]) => `${object}-${id}`,
load: ([object, id]) => true,
});

jest.spyOn(console, "warn").mockImplementation(() => {});

cache.read({}, "one");

expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching("contains a stringified object")
);

// Only warn once per cache though
cache.read({}, "two");
expect(console.warn).toHaveBeenCalledTimes(1);
});
});
});
43 changes: 32 additions & 11 deletions packages/suspense/src/cache/createCache.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isDevelopment } from "#is-development";
import { unstable_getCacheForType as getCacheForTypeMutable } from "react";
import { STATUS_NOT_FOUND, STATUS_PENDING } from "../constants";
import { createDeferred } from "../utils/createDeferred";
import {
Cache,
CacheLoadOptions,
Expand All @@ -11,21 +11,22 @@ import {
StatusCallback,
UnsubscribeCallback,
} from "../types";
import {
createPendingRecord,
createResolvedRecord,
updateRecordToRejected,
updateRecordToResolved,
} from "../utils/Record";
import { assertPendingRecord } from "../utils/assertRecordStatus";
import { createDeferred } from "../utils/createDeferred";
import { defaultGetCache } from "../utils/defaultGetCache";
import { defaultGetKey } from "../utils/defaultGetKey";
import { isPromiseLike } from "../utils/isPromiseLike";
import {
isPendingRecord,
isRejectedRecord,
isResolvedRecord,
} from "../utils/isRecordStatus";
import { defaultGetKey } from "../utils/defaultGetKey";
import { defaultGetCache } from "../utils/defaultGetCache";
import {
createPendingRecord,
createResolvedRecord,
updateRecordToRejected,
updateRecordToResolved,
} from "../utils/Record";

export type InternalCache<Params extends Array<any>, Value> = Cache<
Params,
Expand Down Expand Up @@ -61,11 +62,31 @@ const DEBUG_LOG_IN_DEV = false;
export function createCache<Params extends Array<any>, Value>(
options: CreateCacheOptions<Params, Value>
): Cache<Params, Value> {
const { config = {}, debugLabel, getKey = defaultGetKey, load } = options;
let { config = {}, debugLabel, getKey = defaultGetKey, load } = options;
const { getCache = defaultGetCache, immutable = false } = config;

if (isDevelopment) {
let didLogWarning = false;
let decoratedGetKey = getKey;

getKey = (params: Params) => {
const key = decoratedGetKey(params);

if (!didLogWarning) {
if (key.includes("[object Object]")) {
didLogWarning = true;
console.warn(
`Warning: createCache() key "${key}" contains a stringified object and may not be unique`
);
}
}

return key;
};
}

const debugLogInDev = (debug: string, params?: Params, ...args: any[]) => {
if (DEBUG_LOG_IN_DEV && process.env.NODE_ENV !== "production") {
if (DEBUG_LOG_IN_DEV && isDevelopment) {
const cacheKey = params ? `"${getKey(params)}"` : "";
const prefix = debugLabel ? `createCache[${debugLabel}]` : "createCache";

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { compare as compareBigInt } from "extra-bigint";
import {
STATUS_NOT_FOUND,
STATUS_PENDING,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,36 +1,36 @@
import { isDevelopment } from "#is-development";
import { configure as configureArraySortingUtilities } from "array-sorting-utilities";
import {
configure as configureIntervalUtilities,
Interval,
} from "interval-utilities";
import DataIntervalTree from "node-interval-tree";
import { configure as configurePointUtilities } from "point-utilities";

import {
STATUS_NOT_FOUND,
STATUS_PENDING,
STATUS_REJECTED,
STATUS_RESOLVED,
} from "../../constants";
import {
IntervalCacheLoadOptions,
GetPointForValue,
PendingRecord,
IntervalCache,
IntervalCacheLoadOptions,
PendingRecord,
Record,
StatusCallback,
} from "../../types";
import { assertPendingRecord } from "../../utils/assertRecordStatus";
import { createDeferred } from "../../utils/createDeferred";
import { defaultGetKey } from "../../utils/defaultGetKey";
import { isPromiseLike } from "../../utils/isPromiseLike";
import {
isPendingRecord,
isRejectedRecord,
isResolvedRecord,
} from "../../utils/isRecordStatus";
import { findIntervals } from "./findIntervals";
import { sliceValues } from "./sliceValues";
import { isPromiseLike } from "../../utils/isPromiseLike";

// Enable to help with debugging in dev
const DEBUG_LOG_IN_DEV = false;
Expand Down Expand Up @@ -105,7 +105,7 @@ export function createIntervalCache<
> = new Map();

const debugLogInDev = (debug: string, params?: Params, ...args: any[]) => {
if (DEBUG_LOG_IN_DEV && process.env.NODE_ENV !== "production") {
if (DEBUG_LOG_IN_DEV && isDevelopment) {
const cacheKey = params ? `"${getKey(...params)}"` : "";
const prefix = debugLabel
? `createIntervalCache[${debugLabel}]`
Expand Down
1 change: 0 additions & 1 deletion packages/suspense/src/cache/createIntervalCache/index.ts

This file was deleted.

17 changes: 10 additions & 7 deletions packages/suspense/src/cache/createStreamingCache.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import { isDevelopment } from "#is-development";
import {
STATUS_ABORTED,
STATUS_PENDING,
STATUS_REJECTED,
STATUS_RESOLVED,
} from "../constants";
import { createDeferred } from "../utils/createDeferred";
import {
StreamingCache,
StreamingCacheLoadOptions,
StreamingSubscribeCallback,
StreamingValue,
} from "../types";
import { warnInDev } from "../utils/warnInDev";
import { createDeferred } from "../utils/createDeferred";
import { defaultGetKey } from "../utils/defaultGetKey";

// Enable to help with debugging in dev
Expand All @@ -32,7 +32,7 @@ export function createStreamingCache<
const { debugLabel, getKey = defaultGetKey, load } = options;

const debugLogInDev = (debug: string, params?: Params, ...args: any[]) => {
if (DEBUG_LOG_IN_DEV && process.env.NODE_ENV !== "production") {
if (DEBUG_LOG_IN_DEV && isDevelopment) {
const cacheKey = params ? `"${getKey(...params)}"` : "";
const prefix = debugLabel ? `createCache[${debugLabel}]` : "createCache";

Expand Down Expand Up @@ -159,10 +159,13 @@ export function createStreamingCache<
streamingValues.value = value;

if (progress != null) {
warnInDev(
progress >= 0 && progress <= 1,
`Invalid progress: ${progress}; value must be between 0-1.`
);
if (isDevelopment) {
if (progress < 0 || progress > 1) {
console.warn(
`Invalid progress: ${progress}; value must be between 0-1.`
);
}
}

streamingValues.progress = progress;
}
Expand Down
1 change: 1 addition & 0 deletions packages/suspense/src/env-conditions/development.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const isDevelopment = true;
1 change: 1 addition & 0 deletions packages/suspense/src/env-conditions/production.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const isDevelopment = false;
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { Component, PropsWithChildren } from "react";
import { createRoot } from "react-dom/client";
import { act } from "react-dom/test-utils";
import { createIntervalCache } from "../cache/createIntervalCache";
import { createIntervalCache } from "../cache/createIntervalCache/createIntervalCache";
import {
STATUS_NOT_FOUND,
STATUS_PENDING,
Expand Down
4 changes: 2 additions & 2 deletions packages/suspense/src/hooks/useIntervalCacheStatus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
import { createRoot } from "react-dom/client";
import { act } from "react-dom/test-utils";

import { createIntervalCache } from "../cache/createIntervalCache/createIntervalCache";
import {
STATUS_NOT_FOUND,
STATUS_PENDING,
STATUS_REJECTED,
STATUS_RESOLVED,
} from "../constants";
import { createDeferred } from "../utils/createDeferred";
import { createIntervalCache } from "../cache/createIntervalCache";
import {
Deferred,
IntervalCache,
IntervalCacheLoadOptions,
Status,
} from "../types";
import { createDeferred } from "../utils/createDeferred";
import { useIntervalCacheStatus } from "./useIntervalCacheStatus";

function createContiguousArray(start: number, end: number) {
Expand Down
2 changes: 1 addition & 1 deletion packages/suspense/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export * from "./constants";
export * from "./cache/createCache";
export * from "./cache/createExternallyManagedCache";
export * from "./cache/createIntervalCache";
export * from "./cache/createIntervalCache/createIntervalCache";
export * from "./cache/createSingleEntryCache";
export * from "./cache/createStreamingCache";
export * from "./hooks/useCacheMutation";
Expand Down
7 changes: 0 additions & 7 deletions packages/suspense/src/utils/warnInDev.ts
Original file line number Diff line number Diff line change
@@ -1,7 +0,0 @@
export function warnInDev(expectedCondition: boolean, message: string) {
if (process.env.NODE_ENV !== "production") {
if (!expectedCondition) {
console.warn(message);
}
}
}
Loading

1 comment on commit 3b5999d

@vercel
Copy link

@vercel vercel bot commented on 3b5999d Jun 9, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.