evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.18k stars 1.16k forks source link

ReferenceError in strict mode #3882

Closed wdzeng closed 3 months ago

wdzeng commented 3 months ago

I found cjs files generated from ts files sometimes throws ReferenceError. I found the problem with a project using adm-zip library.

Reproduction

Using Node.js v20.15.0.

Steps

  1. package.json
    {
    "dependencies": {
    "@types/adm-zip": "0.5.5",
    "adm-zip": "0.5.15",
    "esbuild": "0.23.0"
    }
    }
  2. tsconfig.json
    {
    "compilerOptions": {
    "strict": true
    }
    }
  3. test.ts
    import AdmZip from 'adm-zip'
    const zip = new AdmZip()
    zip.addFile('test.txt', Buffer.from('hello world'))
    zip.writeZip('/tmp/test.zip')
  4. Run npm i && npx esbuild test.ts --platform=node --target=node20 --bundle --outfile=/tmp/test.cjs && node /tmp/test.cjs

Expected Result

/tmp/test.cjs generates /tmp/test.zip.

Actual Result

/tmp/test.cjs throws ReferenceError and fails.

ReferenceError: totalEntries is not defined
    at Object.compressToBuffer (/tmp/test.cjs:1790:24)
    at Object.writeZip (/tmp/test.cjs:2629:30)
    at Object.<anonymous> (/tmp/test.cjs:2688:5)

Hints

  1. Running node /path/to/the/following/script.js will succeed.
"use strict";
const AdmZip = require('adm-zip')
const zip = new AdmZip()
zip.addFile('test.txt', Buffer.from('hello world'))
zip.writeZip('/tmp/test.zip')
  1. If we remove "strict": true in the tsconfig.json, the reproduction will give the expected result.
hyrious commented 3 months ago

I believe this issue is due to adm-zip itself: https://github.com/cthackers/adm-zip/issues/526

esbuild emits "use strict"; on top of the out file if it sees strict config in tsconfig.json. If you delete the first line of /tmp/test.cjs, the issue would disappear because in sloppy mode you just can assign to any global variable without a declaration.

wdzeng commented 3 months ago

Oh thanks!