diff --git a/src/__tests__/app.test.tsx b/src/__tests__/app.test.tsx index a8a4781..e2394e2 100644 --- a/src/__tests__/app.test.tsx +++ b/src/__tests__/app.test.tsx @@ -1,7 +1,7 @@ import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { type ChangeEvent } from 'react' -import { SourceMapGenerator } from 'source-map' +import { type ChangeEvent, StrictMode } from 'react' +import { SourceMapConsumer, SourceMapGenerator } from 'source-map' import { describe, expect, test, vi } from 'vitest' import App from '../app.tsx' @@ -219,6 +219,10 @@ describe('source maps', () => { // Mock the click method to test that it gets called by the keydown handler const clickSpy = vi.spyOn(fileUploadButton, 'click') + // Other keys must not open the file selector + fireEvent.keyDown(fileUploadButton, { code: 'KeyA' }) + expect(clickSpy).not.toHaveBeenCalled() + // Fire the keydown event with Enter fireEvent.keyDown(fileUploadButton, { code: 'Enter' }) @@ -329,6 +333,84 @@ describe('source maps', () => { }) }) +describe('source map lifecycle', () => { + // destroy() lives on the prototype of the concrete consumer class, which the + // library does not export, so grab it from a throwaway instance. + async function spyOnConsumerDestroy() { + const consumer = await new SourceMapConsumer(regular.sourcemaps[0].content) + const prototype = Object.getPrototypeOf(consumer) as { destroy: () => void } + consumer.destroy() + return vi.spyOn(prototype, 'destroy') + } + + test('destroys the consumer of a rejected duplicate source map', async () => { + const destroySpy = await spyOnConsumerDestroy() + + render() + const user = userEvent.setup() + + const sourceMapFileInput = screen.getByLabelText(/choose files/i) + const makeFile = () => new File([regular.sourcemaps[0].content], regular.sourcemaps[0].fileName) + + await user.upload(sourceMapFileInput, makeFile()) + await screen.findByRole('list', { name: /sourcemaps list/i }) + expect(destroySpy).not.toHaveBeenCalled() + + await user.upload(sourceMapFileInput, makeFile()) + await waitFor(() => expect(destroySpy).toHaveBeenCalledOnce()) + + const sourcemapList = screen.getByRole('list', { name: /sourcemaps list/i }) + expect(within(sourcemapList).getAllByRole('listitem')).toHaveLength(1) + + destroySpy.mockRestore() + }) + + test('adds a source map only once when identical files are selected together', async () => { + const destroySpy = await spyOnConsumerDestroy() + + render() + const user = userEvent.setup() + + const sourceMapFileInput = screen.getByLabelText(/choose files/i) + const files = [ + new File([regular.sourcemaps[0].content], 'copy-one.js.map'), + new File([regular.sourcemaps[0].content], 'copy-two.js.map'), + ] + + await user.upload(sourceMapFileInput, files) + + const sourcemapList = await screen.findByRole('list', { name: /sourcemaps list/i }) + expect(within(sourcemapList).getAllByRole('listitem')).toHaveLength(1) + expect(destroySpy).toHaveBeenCalledOnce() + + destroySpy.mockRestore() + }) + + test('destroys the consumer exactly once when deleting under StrictMode', async () => { + const destroySpy = await spyOnConsumerDestroy() + + render( + + + , + ) + const user = userEvent.setup() + + const sourceMapFileInput = screen.getByLabelText(/choose files/i) + const file = new File([regular.sourcemaps[0].content], regular.sourcemaps[0].fileName) + await user.upload(sourceMapFileInput, file) + + const deleteButton = await screen.findByRole('button', { name: 'delete' }) + await user.click(deleteButton) + + const sourcemapList = screen.queryByRole('list', { name: /sourcemaps list/i }) + expect(sourcemapList).not.toBeInTheDocument() + expect(destroySpy).toHaveBeenCalledOnce() + + destroySpy.mockRestore() + }) +}) + describe('column numbers', () => { // Generated line 1 has mappings at 0-based columns 0, 10, and 11 which lead // to different original lines, so an off-by-one in the column conversion diff --git a/src/lib.ts b/src/lib.ts index e66992f..3bab27b 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -25,7 +25,7 @@ export function transform(sourceMaps: SourceMap[], stackTrace: null | StackTrace function tryGetOriginalPosition( stackFrame: StackFrame, bindings: Record, -): null | NullableMappedPosition { +): null | OriginalPosition { const { column, file, line } = toUnifiedPosition(stackFrame) if (!file || !bindings[file] || line == null || line < 1 || column == null) { @@ -38,7 +38,21 @@ function tryGetOriginalPosition( line, }) - return result.source == null ? null : result + return isResolvedPosition(result) ? result : null +} + +interface OriginalPosition { + column: number + line: number + name: null | string + source: string +} + +// A found mapping always carries line and column along with the source. +function isResolvedPosition( + position: NullableMappedPosition, +): position is NullableMappedPosition & OriginalPosition { + return position.source != null } function generateStackTraceLine(position: UnifiedPosition) { @@ -46,7 +60,7 @@ function generateStackTraceLine(position: UnifiedPosition) { return ` at${method ? ` ${method}` : ''} (${file}:${line}:${column})` } -function toUnifiedPosition(position: NullableMappedPosition | StackFrame): UnifiedPosition { +function toUnifiedPosition(position: OriginalPosition | StackFrame): UnifiedPosition { if (isStackFrame(position)) { return { column: position.column, @@ -58,14 +72,14 @@ function toUnifiedPosition(position: NullableMappedPosition | StackFrame): Unifi return { // The source-map library returns 0-based columns, stack traces use 1-based. - column: position.column == null ? null : position.column + 1, + column: position.column + 1, file: position.source, line: position.line, method: position.name, } } -function isStackFrame(position: NullableMappedPosition | StackFrame): position is StackFrame { +function isStackFrame(position: OriginalPosition | StackFrame): position is StackFrame { return 'lineNumber' in position } diff --git a/src/use-sourcemaps-store.ts b/src/use-sourcemaps-store.ts index ce18009..079c003 100644 --- a/src/use-sourcemaps-store.ts +++ b/src/use-sourcemaps-store.ts @@ -5,23 +5,36 @@ import { SourceMap } from './source-map.ts' export function useSourcemapsStore() { const [sourceMaps, setSourceMaps] = useState([]) + // Deduplication and consumer destruction happen outside the state updaters: + // updaters must be pure, StrictMode invokes them twice. function addSourceMaps(value: (null | SourceMap)[] | SourceMap) { const candidates = (Array.isArray(value) ? value : [value]).filter( (sm): sm is SourceMap => sm !== null, ) - setSourceMaps(prev => { - const toAdd = candidates.filter(sm => !prev.some(s => s.isEqual(sm))) - return toAdd.length ? [...prev, ...toAdd] : prev - }) + + const toAdd: SourceMap[] = [] + + for (const candidate of candidates) { + if ([...sourceMaps, ...toAdd].some(sm => sm.isEqual(candidate))) { + candidate.consumer.destroy() + } else { + toAdd.push(candidate) + } + } + + if (toAdd.length) { + setSourceMaps(prev => [...prev, ...toAdd]) + } } function deleteSourceMap(id: number) { - setSourceMaps(prev => { - const target = prev.find(sm => sm.id === id) - if (!target) return prev - target.consumer.destroy() - return prev.filter(sm => sm.id !== id) - }) + for (const sourceMap of sourceMaps) { + if (sourceMap.id === id) { + sourceMap.consumer.destroy() + } + } + + setSourceMaps(prev => prev.filter(sm => sm.id !== id)) } return { addSourceMaps, deleteSourceMap, sourceMaps } diff --git a/vite.config.ts b/vite.config.ts index 0a5ecb2..83624da 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -21,6 +21,7 @@ export default defineConfig({ provider: 'v8', reporter: ['text', 'json'], include: ['src/**/*.{ts,tsx}'], + thresholds: { 100: true }, }, }, })