Skip to content
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 .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
registry-url: 'https://registry.npmjs.org'

- name: Install pnpm
uses: pnpm/action-setup@v3
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down
17 changes: 8 additions & 9 deletions demo/package.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
{
"name": "react-inlinesvg-demo",
"version": "4.0.0",
"version": "4.2.0",
"description": "An SVG loader component for ReactJS",
"keywords": [],
"dependencies": {
"loader-utils": "3.2.1",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-inlinesvg": "latest",
"react-scripts": "^5.0.1",
"styled-components": "^6.0.7"
"styled-components": "^6.1.15"
},
"devDependencies": {
"@types/node": "18.15.3",
"@types/react": "18.0.25",
"@types/react-dom": "18.0.9",
"typescript": "4.9.5"
"@types/node": "22.13.4",
"@types/react": "18.3.18",
"@types/react-dom": "18.3.5",
"typescript": "5.7.3"
},
"scripts": {
"start": "react-scripts start",
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@
"@gilbarbara/tsconfig": "^0.2.3",
"@size-limit/preset-small-lib": "^11.1.6",
"@testing-library/jest-dom": "^6.6.3",
"@testing-library/react": "^16.1.0",
"@types/node": "^22.10.6",
"@testing-library/react": "^16.2.0",
"@types/node": "^22.13.4",
"@types/react": "^18.3.18",
"@types/react-dom": "^18.3.5",
"@vitejs/plugin-react": "^4.3.4",
"@vitest/coverage-v8": "^2.1.8",
"@vitest/coverage-v8": "^3.0.6",
"browser-cache-mock": "^0.1.7",
"del-cli": "^6.0.0",
"fix-tsup-cjs": "^1.2.0",
Expand All @@ -79,9 +79,9 @@
"size-limit": "^11.1.6",
"start-server-and-test": "^2.0.10",
"ts-node": "^10.9.2",
"tsup": "^8.3.5",
"tsup": "^8.3.6",
"typescript": "^5.7.3",
"vitest": "^2.1.8",
"vitest": "^3.0.6",
"vitest-fetch-mock": "^0.4.3"
},
"scripts": {
Expand Down
315 changes: 167 additions & 148 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

77 changes: 50 additions & 27 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function ReactInlineSVG(props: Props) {
status: STATUS.READY,
});
} catch (error: any) {
handleError(new Error(error.message));
handleError(error);
}
}, [content, handleError, props]);

Expand Down Expand Up @@ -149,7 +149,7 @@ function ReactInlineSVG(props: Props) {
isActive.current = true;

if (!canUseDOM() || isInitialized.current) {
return () => undefined;
return undefined;
}

try {
Expand Down Expand Up @@ -178,13 +178,9 @@ function ReactInlineSVG(props: Props) {
[],
);

// Handle prop changes
// Handles `src` changes
useEffect(() => {
if (!canUseDOM()) {
return;
}

if (!previousProps) {
if (!canUseDOM() || !previousProps) {
return;
}

Expand All @@ -196,27 +192,55 @@ function ReactInlineSVG(props: Props) {
}

load();
} else if (previousProps.title !== title || previousProps.description !== description) {
}
}, [handleError, load, previousProps, src]);

// Handles content loading
useEffect(() => {
if (status === STATUS.LOADED) {
getElement();
}
}, [description, getElement, handleError, load, previousProps, src, title]);
}, [status, getElement]);

// handle state
// Handles `title` and `description` changes
useEffect(() => {
if (!previousState) {
if (!canUseDOM() || !previousProps || previousProps.src !== src) {
return;
}

if (previousState.status !== STATUS.LOADING && status === STATUS.LOADING) {
getContent();
if (previousProps.title !== title || previousProps.description !== description) {
getElement();
}
}, [description, getElement, previousProps, src, title]);

if (previousState.status !== STATUS.LOADED && status === STATUS.LOADED) {
getElement();
// handle state
useEffect(() => {
if (!previousState) {
return;
}

if (previousState.status !== STATUS.READY && status === STATUS.READY) {
onLoad?.(src, isCached);
switch (status) {
case STATUS.LOADING: {
if (previousState.status !== STATUS.LOADING) {
getContent();
}

break;
}
case STATUS.LOADED: {
if (previousState.status !== STATUS.LOADED) {
getElement();
}

break;
}
case STATUS.READY: {
if (previousState.status !== STATUS.READY) {
onLoad?.(src, isCached);
}

break;
}
}
}, [getContent, getElement, isCached, onLoad, previousState, src, status]);

Expand All @@ -243,7 +267,7 @@ function ReactInlineSVG(props: Props) {
}

if (element) {
return cloneElement(element as ReactElement<any>, {
return cloneElement(element as ReactElement, {
ref: innerRef,
...elementProps,
});
Expand All @@ -262,18 +286,17 @@ export default function InlineSVG(props: Props) {
}

const { loader } = props;
const hasCallback = useRef(false);
const [isReady, setReady] = useState(cacheStore.isReady);

useEffect(() => {
if (!hasCallback.current) {
cacheStore.onReady(() => {
setReady(true);
});

hasCallback.current = true;
if (isReady) {
return;
}
}, []);

cacheStore.onReady(() => {
setReady(true);
});
}, [isReady]);

if (!isReady) {
return loader;
Expand Down
32 changes: 21 additions & 11 deletions src/modules/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,24 @@ export default class CacheStore {
.catch(error => {
// eslint-disable-next-line no-console
console.error(`Failed to open cache: ${error.message}`);
this.cacheApi = undefined;
})
.finally(() => {
this.isReady = true;
this.subscribers.forEach(callback => callback());
// Copy to avoid mutation issues
const callbacks = [...this.subscribers];

// Clear array efficiently
this.subscribers.length = 0;

callbacks.forEach(callback => {
try {
callback();
} catch (error: any) {
// eslint-disable-next-line no-console
console.error(`Error in CacheStore subscriber callback: ${error.message}`);
}
});
});
} else {
this.isReady = true;
Expand Down Expand Up @@ -131,17 +145,16 @@ export default class CacheStore {
}

private async handleLoading(url: string, callback: () => Promise<void>) {
let retryCount = 0;
for (let retryCount = 0; retryCount < CACHE_MAX_RETRIES; retryCount++) {
if (this.cacheStore.get(url)?.status !== STATUS.LOADING) {
return;
}

while (this.cacheStore.get(url)?.status === STATUS.LOADING && retryCount < CACHE_MAX_RETRIES) {
// eslint-disable-next-line no-await-in-loop
await sleep(0.1);
retryCount += 1;
}

if (retryCount >= CACHE_MAX_RETRIES) {
await callback();
}
await callback();
}

public keys(): Array<string> {
Expand All @@ -164,10 +177,7 @@ export default class CacheStore {
if (this.cacheApi) {
const keys = await this.cacheApi.keys();

for (const key of keys) {
// eslint-disable-next-line no-await-in-loop
await this.cacheApi.delete(key);
}
await Promise.allSettled(keys.map(key => this.cacheApi!.delete(key)));
}

this.cacheStore.clear();
Expand Down
37 changes: 34 additions & 3 deletions test/modules/cache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,14 @@ describe('CacheStore (external)', () => {
value: true,
});
const mockReady = vi.fn();
const cacheStore = new CacheStore();
let cacheStore: CacheStore;

// wait for the cache to be ready
cacheStore.onReady(mockReady);
beforeAll(async () => {
// wait for the cache to be ready
cacheStore = new CacheStore();
cacheStore.onReady(mockReady);
await waitFor(() => expect(mockReady).toHaveBeenCalledTimes(1));
});

beforeEach(() => {
fetchMock.mockResponse(() => Promise.resolve(reactContent));
Expand Down Expand Up @@ -198,6 +202,33 @@ describe('CacheStore (external)', () => {
expect(fetchMock).toHaveBeenCalledTimes(0);
});

it('should correctly cache and return empty responses', async () => {
fetchMock.mockResponseOnce(() => Promise.resolve(''));

// First fetch: Expect it to store the empty response
await expect(cacheStore.get(jsUrl)).resolves.toBe('');
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(cacheStore.isCached(jsUrl)).toBeTrue();

expect(cacheStore.data()).toEqual([{ [jsUrl]: { content: '', status: STATUS.LOADED } }]);

// Second fetch: Should return cached empty response without calling fetch
await expect(cacheStore.get(jsUrl)).resolves.toBe('');
expect(fetchMock).toHaveBeenCalledTimes(1); // Should still be 1 if properly cached

// Delete and refetch: Should trigger another fetch
await cacheStore.delete(jsUrl);

expect(cacheStore.isCached(jsUrl)).toBeFalse();
expect(cacheStore.keys()).toHaveLength(0);

fetchMock.mockResponseOnce(() => Promise.resolve(jsContent));

await expect(cacheStore.get(jsUrl)).resolves.toBe(jsContent);

expect(fetchMock).toHaveBeenCalledTimes(2); // Fetch count should increase
});

it('should handle delete', async () => {
await cacheStore.get(reactUrl);

Expand Down
2 changes: 1 addition & 1 deletion test/unsupported.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('unsupported environments', () => {
const { container } = setup();

await waitFor(() => {
expect(mockOnError).toHaveBeenCalledWith(new Error('fetch is not a function'));
expect(mockOnError).toHaveBeenCalledWith(new TypeError('fetch is not a function'));
});

expect(container.firstChild).toMatchSnapshot();
Expand Down