search

ezolenko / rollup-plugin-typescript2

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

`0.34.0` not handling JSON import in `rollup.config.ts` #426

Closed halo951 closed 1 year ago

halo951 commented 1 year ago

Troubleshooting

  1. Does tsc have the same output? If so, please explain why this is incorrect behavior

    no

  2. Does your Rollup plugin order match this plugin's compatibility? If not, please elaborate

    matched

  3. Can you create a minimal example that reproduces this behavior? Preferably, use this environment for your reproduction

https://stackblitz.com/edit/rpt2-repro-h4yxsj?file=rollup.config.ts

What happens and why it is incorrect

In the rollup.config.ts, use the import pkg from 'package.json' statement, and the plug-in does not handle. and it is normal in <=v0.33.0

Environment

  System:
    OS: Windows 10 10.0.17763
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 7.05 GB / 15.71 GB
  Binaries:
    Node: 18.9.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.19.1 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    rollup: ^2.79.1 => 2.79.1
    rollup-plugin-typescript2: 0.33.0 => 0.33.0
    typescript: ^4.8.3 => 4.8.3

Versions

v0.34.0

agilgur5 commented 1 year ago

and it is normal in <=v0.33.0

Thanks for the repro, I was able to successfully reproduce in 0.34.0 and not in 0.33.0, getting this error:

~/projects/rpt2-repro-h4yxsj 1s
❯ npm run build
$ rollup --config ./rollup.config.ts --configPlugin typescript2
[!] (plugin rpt2) Error: Unexpected token (Note that you need @rollup/plugin-json to import JSON files)
package.json (2:8)
1: {
2:   "name": "rpt2-repro",
           ^
3:   "version": "0.0.0",
4:   "scripts": {

    at error (/home/projects/rpt2-repro-h4yxsj/node_modules/rollup/dist/shared/rollup.js:198:30)
    at Module.error (/home/projects/rpt2-repro-h4yxsj/node_modules/rollup/dist/shared/rollup.js:12553:16)
    at Module.tryParse (/home/projects/rpt2-repro-h4yxsj/node_modules/rollup/dist/shared/rollup.js:12930:25)
    at Module.setSource (/home/projects/rpt2-repro-h4yxsj/node_modules/rollup/dist/shared/rollup.js:12835:24)
    at ModuleLoader.addModuleSource (/home/projects/rpt2-repro-h4yxsj/node_modules/rollup/dist/shared/rollup.js:22309:20)

0.34.0 shouldn't have changed this behavior though -- just added more type-checking and declaration generation for type-only files, so I'm actually not sure what broke... Here's the diff: https://github.com/ezolenko/rollup-plugin-typescript2/compare/0.33.0.1...0.34.0#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80L208

I'll have to investigate this more when I get the time, nice catch on this change between 0.33.0 and 0.34.0!

halo951 commented 1 year ago

Thank you for your reply. I went to see what caused the mistake today

agilgur5 commented 1 year ago

So I added some verbosity: 3 logging that was missing from your issue and repro:

 "scripts": {
    "build": "rollup --config ./rollup.config.ts --configPlugin \"typescript2={ verbosity: 3 }\"",
  },

and the strange part is that the logs between versions are virtually identical (outside of version number, cache hash, and the error ofc, everything else is identical):

0.34.0 ``` $ rollup --config ./rollup.config.ts --configPlugin "typescript2={ verbosity: 3 }" rpt2: built-in options overrides: { "noEmitHelpers": false, "importHelpers": true, "noResolve": false, "noEmit": false, "noEmitOnError": false, "inlineSourceMap": false, "outDir": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/placeholder", "moduleResolution": 2, "allowNonTsExtensions": true } rpt2: parsed tsconfig: { "options": { "lib": [ "lib.esnext.d.ts", "lib.dom.d.ts" ], "target": 99, "module": 99, "moduleResolution": 2, "jsx": 1, "sourceMap": false, "noEmit": false, "noEmitHelpers": false, "removeComments": false, "importHelpers": true, "resolveJsonModule": true, "strict": true, "experimentalDecorators": true, "emitDecoratorMetadata": true, "esModuleInterop": true, "declaration": true, "configFilePath": "/home/projects/rpt2-issue-426/tsconfig.json", "noResolve": false, "noEmitOnError": false, "inlineSourceMap": false, "outDir": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/placeholder", "allowNonTsExtensions": true }, "fileNames": [ "/home/projects/rpt2-issue-426/src/index.ts" ], "typeAcquisition": { "enable": false, "include": [], "exclude": [] }, "raw": { "include": [ "src/*.ts" ], "exclude": [], "compilerOptions": { "lib": [ "esnext", "DOM" ], "target": "ESNext", "module": "ESNext", "moduleResolution": "node", "jsx": "preserve", "sourceMap": false, "noEmit": true, "noEmitHelpers": true, "removeComments": false, "importHelpers": true, "resolveJsonModule": true, "strict": true, "experimentalDecorators": true, "emitDecoratorMetadata": true, "esModuleInterop": true, "declaration": true, "declarationDir": "typings" }, "compileOnSave": false }, "errors": [], "wildcardDirectories": { "/home/projects/rpt2-issue-426/src": 0 }, "compileOnSave": false } rpt2: typescript version: 4.7.3 rpt2: tslib version: 2.4.0 rpt2: rollup version: 2.75.6 rpt2: rollup-plugin-typescript2 version: 0.34.0 rpt2: plugin options: { "check": true, "verbosity": 3, "clean": false, "cacheRoot": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2", "include": [ "*.ts+(|x)", "**/*.ts+(|x)" ], "exclude": [ "*.d.ts", "**/*.d.ts" ], "abortOnError": true, "rollupCommonJSResolveHack": false, "useTsconfigDeclarationDir": false, "tsconfigOverride": {}, "transformers": [], "tsconfigDefaults": {}, "objectHashIgnoreUnknownHack": false, "cwd": "/home/projects/rpt2-issue-426", "typescript": "version 4.7.3" } rpt2: rollup config: { "input": "/home/projects/rpt2-issue-426/rollup.config.ts", "plugins": [ { "name": "rpt2" } ], "treeshake": false } rpt2: tsconfig path: /home/projects/rpt2-issue-426/tsconfig.json rpt2: included: [ "*.ts+(|x)", "**/*.ts+(|x)" ] rpt2: excluded: [ "*.d.ts", "**/*.d.ts" ] rpt2: Ambient types: rpt2: transpiling '/home/projects/rpt2-issue-426/rollup.config.ts' rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_5216657e82e17d90a7ac1f26c207b61ce8c9785f/code/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_5216657e82e17d90a7ac1f26c207b61ce8c9785f/syntacticDiagnostics/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_5216657e82e17d90a7ac1f26c207b61ce8c9785f/semanticDiagnostics/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: generated declarations for '/home/projects/rpt2-issue-426/rollup.config.ts' rpt2: rolling caches [!] (plugin rpt2) Error: Unexpected token (Note that you need @rollup/plugin-json to import JSON files) package.json (2:8) 1: { 2: "name": "rpt2-repro", ^ 3: "version": "0.0.0", 4: "scripts": { at error (/home/projects/rpt2-issue-426/node_modules/rollup/dist/shared/rollup.js:198:30) at Module.error (/home/projects/rpt2-issue-426/node_modules/rollup/dist/shared/rollup.js:12553:16) at Module.tryParse (/home/projects/rpt2-issue-426/node_modules/rollup/dist/shared/rollup.js:12930:25) at Module.setSource (/home/projects/rpt2-issue-426/node_modules/rollup/dist/shared/rollup.js:12835:24) at ModuleLoader.addModuleSource (/home/projects/rpt2-issue-426/node_modules/rollup/dist/shared/rollup.js:22309:20) ```
0.33.0 ``` $ rollup --config ./rollup.config.ts --configPlugin "typescript2={ verbosity: 3 }" rpt2: built-in options overrides: { "noEmitHelpers": false, "importHelpers": true, "noResolve": false, "noEmit": false, "noEmitOnError": false, "inlineSourceMap": false, "outDir": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/placeholder", "moduleResolution": 2, "allowNonTsExtensions": true } rpt2: parsed tsconfig: { "options": { "lib": [ "lib.esnext.d.ts", "lib.dom.d.ts" ], "target": 99, "module": 99, "moduleResolution": 2, "jsx": 1, "sourceMap": false, "noEmit": false, "noEmitHelpers": false, "removeComments": false, "importHelpers": true, "resolveJsonModule": true, "strict": true, "experimentalDecorators": true, "emitDecoratorMetadata": true, "esModuleInterop": true, "declaration": true, "configFilePath": "/home/projects/rpt2-issue-426/tsconfig.json", "noResolve": false, "noEmitOnError": false, "inlineSourceMap": false, "outDir": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/placeholder", "allowNonTsExtensions": true }, "fileNames": [ "/home/projects/rpt2-issue-426/src/index.ts" ], "typeAcquisition": { "enable": false, "include": [], "exclude": [] }, "raw": { "include": [ "src/*.ts" ], "exclude": [], "compilerOptions": { "lib": [ "esnext", "DOM" ], "target": "ESNext", "module": "ESNext", "moduleResolution": "node", "jsx": "preserve", "sourceMap": false, "noEmit": true, "noEmitHelpers": true, "removeComments": false, "importHelpers": true, "resolveJsonModule": true, "strict": true, "experimentalDecorators": true, "emitDecoratorMetadata": true, "esModuleInterop": true, "declaration": true, "declarationDir": "typings" }, "compileOnSave": false }, "errors": [], "wildcardDirectories": { "/home/projects/rpt2-issue-426/src": 0 }, "compileOnSave": false } rpt2: typescript version: 4.7.3 rpt2: tslib version: 2.4.0 rpt2: rollup version: 2.75.6 rpt2: rollup-plugin-typescript2 version: 0.33.0 rpt2: plugin options: { "check": true, "verbosity": 3, "clean": false, "cacheRoot": "/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2", "include": [ "*.ts+(|x)", "**/*.ts+(|x)" ], "exclude": [ "*.d.ts", "**/*.d.ts" ], "abortOnError": true, "rollupCommonJSResolveHack": false, "useTsconfigDeclarationDir": false, "tsconfigOverride": {}, "transformers": [], "tsconfigDefaults": {}, "objectHashIgnoreUnknownHack": false, "cwd": "/home/projects/rpt2-issue-426", "typescript": "version 4.7.3" } rpt2: rollup config: { "input": "/home/projects/rpt2-issue-426/rollup.config.ts", "plugins": [ { "name": "rpt2" } ], "treeshake": false } rpt2: tsconfig path: /home/projects/rpt2-issue-426/tsconfig.json rpt2: included: [ "*.ts+(|x)", "**/*.ts+(|x)" ] rpt2: excluded: [ "*.d.ts", "**/*.d.ts" ] rpt2: Ambient types: rpt2: transpiling '/home/projects/rpt2-issue-426/rollup.config.ts' rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_d73542ac828823245b509e8d3cc925f8622f2a68/code/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_d73542ac828823245b509e8d3cc925f8622f2a68/syntacticDiagnostics/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_d73542ac828823245b509e8d3cc925f8622f2a68/semanticDiagnostics/cache/c2337bbe99a2b90fbcf374b0631147a55d2797d4' rpt2: cache miss rpt2: generated declarations for '/home/projects/rpt2-issue-426/rollup.config.ts' rpt2: type-checking missed '/home/projects/rpt2-issue-426/src/index.ts' rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_d73542ac828823245b509e8d3cc925f8622f2a68/syntacticDiagnostics/cache/1a3ed5610e1699ca5a960e962f6b1fb0c3a1c5cb' rpt2: cache miss rpt2: cache: '/home/projects/rpt2-issue-426/node_modules/.cache/rollup-plugin-typescript2/rpt2_d73542ac828823245b509e8d3cc925f8622f2a68/semanticDiagnostics/cache/1a3ed5610e1699ca5a960e962f6b1fb0c3a1c5cb' rpt2: cache miss rpt2: rolling caches rpt2: generating target 1 rpt2: generating missed declarations for '/home/projects/rpt2-issue-426/src/index.ts' rpt2: generated declarations for '/home/projects/rpt2-issue-426/src/index.ts' rpt2: emitting declarations for '/home/projects/rpt2-issue-426/rollup.config.ts' to 'rollup.config.d.ts' rpt2: emitting declarations for '/home/projects/rpt2-issue-426/src/index.ts' to 'src/index.d.ts' ./src/index.ts → dist/index.cjs.js... created dist/index.cjs.js in 273ms ```

There is also no "resolving" being done in either log (and the resolveId hook did not change in 0.34.0), so this seems to be hit in the transform hook.

The strange part there is that rpt2 shouldn't be transforming any JSON, since it's not in the default include/exclude. So upon first glance, the error seems correct. But it doesn't appear in 0.33.0 for some reason. 🤔

I have a feeling this is an odd edge-case for configPlugins in particular, where Rollup might handle JSON for them (or just not touch JSON at all, since configPlugins run in Node, which can interpret JSON itself). 0.34.0 forces a load of all references (this should be all imports basically) though, so maybe this is triggering an edge-case bug in Rollup itself because of that. I'll have to do a bit more investigating and add additional logging to confirm

agilgur5 commented 1 year ago

I have a feeling this is an odd edge-case for configPlugins in particular, where Rollup might handle JSON for them (or just not touch JSON at all, since configPlugins run in Node, which can interpret JSON itself). 0.34.0 forces a load of all references (this should be all imports basically) though, so maybe this is triggering an edge-case bug in Rollup itself because of that. I'll have to do a bit more investigating and add additional logging to confirm

I did some additional logging and experimentation, and this hunch of mine seems to be roughly correct: the additional this.load that was added in 0.34.0 to handle type-only files also runs on this JSON file. Skipping it for this file makes the error no longer appear.

We could potentially skip this.load on JSON files, but this seems like a possible (likely) bug in Rollup's this.load implementation, where it's not quite handling JSON the same way for configPlugins as the Rollup CLI does. The addition of this.load shouldn't have changed any behavior as Rollup should have loaded this file either way, as it's present in the JS. So that's why this seems like a Rollup bug to me.

For reference, here's the JS that rpt2 outputs:

import typescript from 'rollup-plugin-typescript2';
import pkg from './package.json';
const banner = `/** ${pkg.name} */`;
export default () => {
    return {
        input: './src/index.ts',
        output: {
            banner,
            exports: 'auto',
            inlineDynamicImports: true,
            format: 'cjs',
            file: `dist/index.cjs.js`,
        },
        plugins: [
            typescript({
                clean: true,
                useTsconfigDeclarationDir: true,
                abortOnError: true,
            }),
        ],
    };
};

This is correct, so that suggests this isn't an rpt2 bug, but an upstream Rollup bug.

I'm also not 100% sure that rpt2 skipping JSON files would be valid behavior. A theoretical scenario, for instance, would be if another plugin were to transform the JSON to a type-only TS file, which would be a case rpt2 should handle. I need to think a bit more on this as this scenario might actually still be handled (in a different way)... 🤔

agilgur5 commented 1 year ago

This is a very hacky workaround, but could also do:

const pkg = JSON.parse(await fs.readFile('./package.json', 'utf8'));

as this SO answer does (for a more detailed example).

(or ofc, add @rollup/plugin-json as another configPlugin)

agilgur5 commented 1 year ago

I'm also not 100% sure that rpt2 skipping JSON files would be valid behavior. A theoretical scenario, for instance, would be if another plugin were to transform the JSON to a type-only TS file, which would be a case rpt2 should handle. I need to think a bit more on this as this scenario might actually still be handled (in a different way)... 🤔

I dug into this a bit, in particular, re-reading my initial logic and root cause analysis of the type-only issue, and I actually think this should be feasible.

Based on my logic there, this.resolve / this.load was mainly necessary so that all the imports (references in TS) go through all plugins + externals and for recursive purposes. So, if I added some preemptive filtering, that should be fine, so long as the this.resolve / this.load still happen afterward. Basically, this.resolve should only be removing references, so removing more preemptively shouldn't break that logic

So I think adding a fix to filter early should be ok.

I also use rpt2 as a configPlugin and import package.json in my rollup.config.ts, so I think that's a pretty common (and personal) use-case to ensure works. Might also add some dogfooding here for that.

agilgur5 commented 1 year ago

Looks like this has been coincidentally fixed by #451. By coincidence, this line ends up fixing this regression as well because it makes the exact same fix I outlined above 😅 . Per https://github.com/ezolenko/rollup-plugin-typescript2/issues/446#issuecomment-1625859100, I actually made a very similar change in October, but unfortunately had pretty significant trouble getting automated configPlugin tests to work (via dogfooding in this commit), so as a result, unfortunately never completed that work 😞

Running the repro with 0.35.0 has no more errors, so marking this as resolved now 🙂

Will still be looking to merge in part of my branch there in order to automatically test and future-proof the configPlugin functionality.