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
1 change: 1 addition & 0 deletions src/ConversionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type SheetLink = {
name: string;
rId: string;
index: number;
hidden: 0 | 1 | 2; // 0: visible, 1: hidden, 2: very hidden
};

type RefLink = {
Expand Down
16 changes: 16 additions & 0 deletions src/convertBinary.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,20 @@ describe('convertBinary', () => {
expect(jsf.images[imgName].slice(0, 32)).toBe('data:image/png;base64,iVBORw0KGg');
});
});

test('images from all sheets are collected (not lost to concurrent processing)', async () => {
// charts-and-images.xlsx has two sheets:
// Sheet1: background picture (image5.png) + drawing with 3 charts
// Sheet2: drawing containing image6.png
// Both images should appear in wb.images. A race condition in the
// Promise.all sheet processing loop can cause one sheet's images to
// overwrite another's because context.images is shared mutable state.
const bin = await readFile('./tests/excel/charts-and-images.xlsx');
const jsf = await convertBinary(bin, 'charts-and-images.xlsx');
const imageKeys = Object.keys(jsf.images ?? {}).sort();
expect(imageKeys).toStrictEqual([
'xl/media/image5.png',
'xl/media/image6.png',
]);
});
});
10 changes: 6 additions & 4 deletions src/convertBinary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ export async function convertBinary (
const styleDefs = await maybeRead(context, 'styles', handlerStyles);
wb.styles = convertStyles(styleDefs);

// worksheets
await Promise.all(context.sheetLinks.map(async (sheetLink, index) => {
// worksheets — processed sequentially to avoid shared-state races on
// context.images and context.drawingRels between await points
for (const [ index, sheetLink ] of context.sheetLinks.entries()) {
const sheetRel = context.rels.find(d => d.id === sheetLink.rId);
if (sheetRel) {
const sheetName = sheetLink.name || `Sheet${sheetLink.index}`;
Expand Down Expand Up @@ -274,14 +275,15 @@ export async function convertBinary (
}
}
if (imageCount) {
wb.images = images;
wb.images ??= {};
Object.assign(wb.images, images);
}
}
}
else {
// TODO: add strict mode that: throw new Error('No rel found for sheet ' + sheetLink.rId);
}
}));
}

// Store people from the workbook.
if (people.length > 0) {
Expand Down
10 changes: 10 additions & 0 deletions src/handler/workbook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,20 @@ export function handlerWorkbook (dom: Document, context: ConversionContext): Wor

dom.querySelectorAll('sheets > sheet')
.forEach(d => {
let hidden: 0 | 1 | 2 = 0;
switch (attr(d, 'state')) {
case 'hidden':
hidden = 1;
break;
case 'veryHidden':
hidden = 2;
break;
}
context.sheetLinks.push({
name: attr(d, 'name'),
index: numAttr(d, 'sheetId'),
rId: attr(d, 'r:id'),
hidden,
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/handler/worksheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function handlerWorksheet (
},
// drawings: [],
// showGridLines: true,
hidden: 0,
hidden: context.sheetLinks.find(link => link.name === sheetName)?.hidden ?? 0,
};

const sheetViews = dom.querySelectorAll('sheetViews > sheetView');
Expand Down
Loading