Skip to content

build: remove unused image-size dependency - #1529

Open
oanaOM wants to merge 1 commit into
gitbrent:masterfrom
oanaOM:remove-unused-image-size-dep
Open

build: remove unused image-size dependency#1529
oanaOM wants to merge 1 commit into
gitbrent:masterfrom
oanaOM:remove-unused-image-size-dep

Conversation

@oanaOM

@oanaOM oanaOM commented Aug 23, 2026

Copy link
Copy Markdown

Submission Guidelines

  • Only modify the src/*.ts files (do not submit dist or src/bld files) — no dist or src/bld files included; this is a manifest-only change (package.json + package-lock.json), matching the shape of the previous dependency commit 9b1b8b8 ("updated image-size for pull bump image-size to 1.2.1 #1387")
  • New and updated properties must be added to src/core-interfaces.ts and types/index.d.tsN/A, no properties added or changed
  • New and updated features must be included in the corresponding demos/modules/*.mjs file — N/A, not a feature

Change Summary

Removes image-size from dependencies. It is declared as a runtime dependency but never imported.

Change Description

The only reference to it in the codebase is inside getSizeFromImage() in src/gen-media.ts, which is commented out and marked unused:

/**
 * FIXME: TODO: currently unused
 * TODO: Should return a Promise
 */
/*
function getSizeFromImage (inImgUrl: string): { width: number, height: number } {
    const sizeOf = typeof require !== 'undefined' ? require('sizeof') : null // NodeJS

Note that even if that block were re-enabled it would not resolve image-size — it requires 'sizeof', which is not in the dependency tree at all.

The browser field stub ("image-size": false) is removed alongside it, since it becomes dead config once the dependency is gone.

No new npm libraries are needed; this only removes one.

Change Type

  • Bug fix (packaging — declared dependency that is never used)
  • New feature
  • Documentation update

Related Issue

None. Raised from downstream dependency-audit findings — see Motivation below.

Motivation and Context

image-size currently has no patched release. Two HIGH advisories:

Both cover <= 2.0.2, and 2.0.2 is the latest published version. There is nothing to upgrade to.

The practical effect is that every pptxgenjs install surfaces two unfixable HIGH findings for code that never executes, and npm audit fix --force "resolves" them by proposing a major downgrade of pptxgenjs itself:

fix available via `npm audit fix --force`
Will install pptxgenjs@1.1.5, which is a breaking change
node_modules/image-size
  pptxgenjs  1.1.5-1 || >=1.1.6
  Depends on vulnerable versions of image-size

That leaves downstream consumers either accepting the risk with a written justification or suppressing the alerts. Dropping the unused declaration removes the finding at its source instead.

For the record, the advisories are not false positives — calling imageSize() directly on a crafted ICNS buffer with a zero-valued entry length hangs the process indefinitely. The flaw is real; it is simply unreachable through pptxgenjs.

Checklist before requesting a review

  • If it is a core feature, I have added new code under /demos/modules/ — N/A, not a feature
  • My code follows the style guidelines of this project — no code changed
  • My changes generate no new eslint warnings — no src files touched
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas — N/A, no code changed
  • I have included code/tests that prove my fix is effective — see verification below
  • I have used the "Run All Demos" feature — ran the Node demo suite (npm run demo-all) rather than the browser demo, since this change only affects Node dependency resolution; details below

Verification

1. Build output is byte-identical. I md5'd src/bld/pptxgen.js, pptxgen.cjs.js and pptxgen.es.js on master, applied the change, reinstalled and rebuilt — all three hashes match:

1161a0a237b349fc952492f456ad7ad7  pptxgen.es.js
40ce5df101f352f5a5b4dcb0d259781e  pptxgen.cjs.js
194032fcbd440cf2145df1aa40d8a164  pptxgen.js

Nothing references image-size, so rollup's external: [...Object.keys(pkg.dependencies)] had nothing to externalize.

2. grep -c "image-size" is 0 across all three built bundles, both before and after.

3. Full Node demo suite passes with image-size absent from the tree. Installed the modified local package into demos/node (confirmed node_modules/image-size absent), then ran npm run demo-all, which exercises Master, Chart, Image, Media, Shape, Text and Table:

* pptxgenjs ver: 4.0.1
--------------------==~==~==~==[ ...DEMO COMPLETE ]==~==~==~==--------------------
EX1 exported: PptxGenJS_Demo_Master,Chart,Image,Media,Shape,Text,Table_...pptx

Completed with no errors and a valid deck. The Image module in particular runs fine without the package present.

Note

https: ^1.0.0 in dependencies looks similarly vestigial — it also has a browser: false stub and appears unused. I left it out to keep this PR focused, but happy to address it separately if useful.

image-size is declared as a runtime dependency but never imported. Its
only reference is inside getSizeFromImage() in src/gen-media.ts, which is
commented out and marked "FIXME: TODO: currently unused" - and which
requires 'sizeof' rather than 'image-size' in any case, a package that is
not in the dependency tree at all.

Removing it leaves the emitted bundles byte-identical (verified by md5 of
src/bld/pptxgen.js, pptxgen.cjs.js and pptxgen.es.js before and after),
since nothing references it for rollup to externalize. The browser field
stub is dropped alongside it as it becomes dead config.

This matters to consumers because image-size currently has no patched
release: GHSA-w3rx-r6r6-pgpr (ICNS) and GHSA-5p2g-fcmc-qvqq (JXL/HEIF)
both cover <= 2.0.2, and 2.0.2 is the latest published version. Every
pptxgenjs install therefore surfaces two unfixable HIGH advisories for
code that is never executed, and `npm audit fix --force` proposes
downgrading pptxgenjs to 1.1.5 to resolve them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lofcz

lofcz commented Aug 24, 2026

Copy link
Copy Markdown

Hey, this is already fixed in pptxgenjs-plus - switching to a maintained fork of pptxgenjs is one line.

Change dependency:

npm uninstall pptxgenjs
npm i pptxgenjs-plus

And update your imports:

- import pptxgen from "pptxgenjs"
+ import pptxgen from "pptxgenjs-plus"

@amitgupte

Copy link
Copy Markdown

@gitbrent Could you complete this PR it causing NPM audit hits in workflows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants