ezolenko / rollup-plugin-typescript2

Rollup plugin for typescript with compiler errors.
MIT License
822 stars 71 forks source link

v0.34.0 - `TypeError: Cannot read property 'done' of undefined` (`cache.done()`) #421

Closed jschill closed 2 years ago

jschill commented 2 years ago

What happens and why it is incorrect

On v0.34 I get: TypeError: Cannot read property 'done' of undefined ...and it points to cache.done() in index.ts that was recently changed.

Downgrading to ~v0.33.0.1~ v0.32 "solves" the problem.

Environment

Versions

System:
    OS: macOS 12.5.1
    CPU: (4) x64 Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz
    Memory: 103.09 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 14.20.0 - ~/.nvm/versions/node/v14.20.0/bin/node
    Yarn: 1.22.18 - ~/.yarn/bin/yarn
    npm: 6.14.17 - ~/.nvm/versions/node/v14.20.0/bin/npm
    Watchman: 2022.08.29.00 - /usr/local/bin/watchman
  npmPackages:
    rollup: 2.79.0 => 2.79.0
    rollup-plugin-typescript2: ^0.34.0 => 0.34.0

rollup.config.js

:
```js import ts from 'rollup-plugin-typescript2'; export default { input: 'src/client/index.ts', plugins: [ ts({ tsconfig: './tsconfig.json', check: false, verbosity: 3 })], output: { dir: 'output', format: 'cjs' }, }; ```

tsconfig.json

:
```json5 { "extends": "../tsconfig.base.json", "transpileOnly": true, "compilerOptions": { "rootDir": "./..", "baseUrl": "./src", "allowJs": true, "target": "es3", "jsx": "react", "jsxFactory": "h", "jsxFragmentFactory": "Fragment", "noLib": false, "outDir": "build/", "noEmitHelpers": true, "preserveConstEnums": true, "suppressImplicitAnyIndexErrors": true, "importHelpers": true, "noUnusedLocals": true, "noUnusedParameters": true, "sourceMap": true, "inlineSources": true, "allowSyntheticDefaultImports": true }, "include": [ "src/**/*.js", "src/**/*.jsx", "src/**/*.ts", "src/**/*.tsx" ], "exclude": [ "node_modules", "**/*.spec.ts" ] } ```

package.json

:
```json ```

plugin output with verbosity 3

:
```text npm run rollup:build > @ rollup:build /Users/johannes/Documents/workspace/sirjunkalot/packages/web > rollup --config rollup.config.js src/client/index.ts → output... [!] (plugin rpt2) TypeError: Cannot read property 'done' of undefined TypeError: Cannot read property 'done' of undefined at buildDone (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup-plugin-typescript2/src/index.ts:82:9) at Object.buildEnd (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup-plugin-typescript2/src/index.ts:295:5) at /Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/shared/rollup.js:22848:40 at async Promise.all (index 0) at PluginDriver.hookParallel (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/shared/rollup.js:22776:9) at /Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/shared/rollup.js:23744:13 at catchUnfinishedHookActions (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/shared/rollup.js:23216:20) at rollupInternal (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/shared/rollup.js:23734:5) at build (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/bin/rollup:1528:20) at runRollup (/Users/johannes/Documents/workspace/sirjunkalot/packages/web/node_modules/rollup/dist/bin/rollup:1668:21) ```
agilgur5 commented 2 years ago

Thanks for the report @jschill .

Per the issue template, could you provide a reproduction of this error?

All tests pass (and I've used 0.34.0 in a few projects already), so this sounds like it must be an edge case that isn't covered by the test suites (yet).

The only way for cache to be undefined is if you hit some error during the buildStart hook, which normally never has errors as it's just an initializer. Your logs point to this line as well, which is an error case conditional.

This would be a simple fix, but it would be good to have a repro as this is an unexpected edge case, meaning we potentially have a bug or something in buildStart too

Downgrading to ~v0.33.0.1~ v0.32 "solves" the problem.

Also is there a reason why 0.33.0 didn't work for you? Since this change is only in 0.34.0

jschill commented 2 years ago

Hello @agilgur5

Per the issue template, could you provide a reproduction of this error?

My project is quite large so there's no easy way for me to create a reproduction. The tl;dr is that I'm trying to change bundler due to Im tired of 1.30m builds, so I'm evaluating different ones and when I came to rollup with this plugin it increased my build time to 2.30m so I kind of moved on pretty quickly :-) If I get some time over I can try to revert and reproduce it.

Downgrading to ~v0.33.0.1~ v0.32 "solves" the problem.

Also is there a reason why 0.33.0 didn't work for you? Since this change is only in 0.34.0

First I tried 0.34.0 - did not work. Downgraded to 0.31.0 I think and then upgraded version by version and everything worked* until 0.34.0, including 0.33.0.1. Then downgraded to 0.33.0.1 again and for some reason it did not work this time. Maybe cache issue or a classic node you-need-to-reinstall-all-node_modules. Continued the downgrade to 0.32.0 and then it started to work again so that's why I changed my error report.

*Oh! It should probably also be mentioned that WHEN it "worked" again it didn't actually work, because I got another error saying I needed to change "module" to "ES2015" or "ESNext" because rollup doesn't support "commonjs". Maybe that is the edge case we're looking for.

agilgur5 commented 2 years ago

The tl;dr is that I'm trying to change bundler due to Im tired of 1.30m builds, so I'm evaluating different ones and when I came to rollup with this plugin it increased my build time to 2.30m so I kind of moved on pretty quickly :-)

Gotcha. I'd recommend trying Vite or Parcel if performance is what you're looking for. And, more generically, probably use esbuild or swc instead of a plugin based on the TypeScript Compiler, which will most certainly be slower.

rpt2, in particular, prefers correctness over performance by default, as you probably noticed from setting check: false (other optimizations include tsconfig declaration: false, skipLibCheck: true, etc -- can check the "optimization" label here).

*Oh! It should probably also be mentioned that WHEN it "worked" again it didn't actually work, because I got another error saying I needed to change "module" to "ES2015" or "ESNext" because rollup doesn't support "commonjs". Maybe that is the edge case we're looking for.

Ah yep, that's the edge case indeed! Thanks for that bit of info, very helpful!

That comes from this line, which is indeed called during initialization in the buildStart hook, so that adds up!

And that line also got changed in another commit in this recent release from a simple throw statement to a Rollup context.error (previously this wasn't possible), which will then trigger the buildEnd error edge case... and hit this bug 🐞

I actually wrote unit tests for this specific tsconfig error, but not an integration test that would capture the interaction b/t these two hooks.

Appreciate the helpful passing-by bug report @jschill 🙂

jschill commented 2 years ago

Gotcha. I'd recommend trying Vite or Parcel if performance is what you're looking for. And, more generically, probably use esbuild or swc instead of a plugin based on the TypeScript Compiler, which will most certainly be slower.

:-) That was my plan as well and that's actually where I'm coming from. Parcel gave me lots of problems since i have a "poor mans monorepo"-setup with nested node_modules. The vite-integration worked but I wasn't happy with the integration to get SSR working. Plus since they don't bundle in dev-mode and deliver es modules - and I haven't implemented code splitting - on page load there were 690+ requests done so time to render was roughly 20 seconds. That DX is horrible, so I need to implement code splitting and that is (hopefully) out-of-scope for what I'm trying to achieve. Might go back and try one of those tools if I don't find another solution.

Appreciate the helpful passing-by bug report @jschill 🙂

👍

agilgur5 commented 2 years ago

Added a 1 char fix for this in #422, as well as a new integration test for this edge case 🙂

agilgur5 commented 2 years ago

Released in 0.34.1