ezolenko / rollup-plugin-typescript2

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

support newer `moduleResolution` kinds, update build to TS 5.x #453

Closed ezolenko closed 1 year ago

ezolenko commented 1 year ago

Summary

Fixes #437

Details

juliaschiller150 commented 11 months ago

Hey team! Thanks for this. Wondering when it might get released / published to npm?

ezolenko commented 11 months ago

@tcschiller could you test to see if this PR works for your use-case?

juliaschiller150 commented 11 months ago

@tcschiller could you test to see if this PR works for your use-case?

Pulled down latest from the master branch and ran the following:

npm ci npm run build npm run build-self npm run build-self npm pack

Then I tried my own repo, first, without that tarball. Built my library and successfully reproduced the problem at hand - it can't resolve imports with relative paths (e.g., import { myExport } from '@scope/package-name/submodule').

Then I ran in my own repo:

npm install ../rollup-plugin-typescript2/rollup-plugin-typescript2-0.35.1.tgz

And immediately retried the build which, up to this point, was failing.

Could no longer reproduce the problem; it seems the MR discussed here fixes everything (because my TypeScript config has the bundler value for moduleResolution).

Thanks for the hard work, @ezolenko . Hope the above proves helpful!

agilgur5 commented 11 months ago

Thanks for testing this out @tcschiller!

For reference, you can reference a GH repo directly in NPM (and any git repo as well) and use a SHA. This commit in particular made a change to the dist folder in the repo, so it already has the compiled version. Very useful for exactly this kind of scenario 🙂

agilgur5 commented 11 months ago

I was also curious about node16 / nodeNext above, specifically whether we should force bundler on those b/c of the file extension requirements.

I approved a few months ago since this is still better than the previous state, but that was the one remaining question on this PR

ezolenko commented 11 months ago

Well, shouldn't break anything. I'll release this sometime tomorrow.

ezolenko commented 11 months ago

In 0.36.0 now

PetarKirov commented 10 months ago

@ezolenko with 0.36.0 I'm getting the following error:

[!] (plugin rpt2) Error: Incompatible tsconfig option. Module resolves to 'Node16'. This is incompatible with Rollup, please use 'module: "ES2015"', 'module: "ES2020"', 'module: "ES2022"', or 'module: "ESNext"'.

With the following tsconfig settings:

{
  "compilerOptions": {
    "module": "Node16",
    "moduleResolution": "Node16",
    // ...
  }
}

And the following Rollup plugin config settings:

  const fileExtensions = ['.mjs', '.cjs', '.js', '.ts', '.json'];

  const plugins = [
    json(),
    commonjs(),

    peerDepsExternal({
      packageJsonPath: path.join(dir, 'package.json'),
    }),
    nodeResolve({
      extensions: fileExtensions,
      preferBuiltins: true,
      exportConditions: ['node'],
      mainFields: ['module', 'main'], // This instructs Rollup to prioritize ESM over CJS when resolving modules
      modulesOnly: true,
    }),
    typescript({
      check: true,
      tsconfig: path.join(dir, 'tsconfig.lib.json'),
      useTsconfigDeclarationDir: true,
    }),
    del({ targets: `${dir}/dist/*` }),
  ];

I'm using the package following versions:

As far as I can see, this comes from these lines:

https://github.com/ezolenko/rollup-plugin-typescript2/blob/0.36.0/src/parse-tsconfig.ts#L46-L48

agilgur5 commented 10 months ago

@PetarKirov this PR only changed the moduleResolution options, not the module options.

There are potentially additional changes needed for the module options, but for the most part, we'd likely just be overriding it to ESNext or something. As Rollup requires ESM throughout, CJS output would be incompatible (TS can output CJS for module node16 and nodenext). So the error is more or less still accurate.

If you need node16 for other tooling, you can use tsconfigOverride to set esnext or es2020 for rpt2 specifically.

raphael-theriault-swi commented 10 months ago

This doesn't work with either node16 or nodenext since they require the module field to match