Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Rollup TS Plugin to '@wessberg/rollup-plugin-ts' to support async rollup plugins #200

Closed
medelman17 opened this issue Sep 8, 2019 · 4 comments · Fixed by #208
Closed
Labels
topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 topic: v0.10.x Issues related to broken builds in v0.10.x / @wessberg/rollup-plugin-ts

Comments

@medelman17
Copy link
Contributor

medelman17 commented Sep 8, 2019

Current Behavior

TSDX uses rollup-plugin-typescript2 to compile and check TypeScript files. Due to certain anomalies specific to this plugin, tsdx users are unable, inter alia, to use additional rollup plugins that contain async/await syntax (e.g., rollup-plugin-visualizer) without a workaround. See ezolenko/rollup-plugin-typescript2#105; see also #183 (comment).

Unfortunately, the workaround, aptly titled, objectHashIgnoreUnknownHack, 'can potentially make cache stale,' due to the fact that rollup-plugin-typescript2 uses a hash of the entire rollup configuration to facilitate cache invalidation. See ezolenko/rollup-plugin-typescript2#105 (comment). As a workaround for the workaround, users are encouraged to bypass the cache completely by setting clean to true in their rollup configurations. See ezolenko/rollup-plugin-typescript2#105 (comment).

The plugin uses rollup config as part of cache key. object-hash is used to generate a hash, but it can't hash certain elements at the moment. Setting this option to true will make object-hash ignore unknowns, at the cost of not invalidating the cache if ignored elements are changed. Only enable this if you need it (Error: Unknown object type "asyncfunction" for example) and make sure to run with clean: true once in a while and definitely before a release. (See #105)

Desired Behavior

As a tsdx user, I should be able to extend tsdx's rollup configuration to include additional plugins that use async/await syntax without a workaround and, especially, without having to worry whether my cache is stale.

Suggested Solution

Replace rollup-plugin-typescript2 with rollup-plugin-ts.

Who does this impact? Who is this for?

While #183 added support for extending tsdx's rollup configuration, making it easy to add additional rollup-plugins, there is no easy way to swap out tsdx's default plugin choices. See, e.g., infra, Alternative Solutions. Thus, tsdx users are, more or less, blocked from using any rollup plugin that contains async/await code. As the async/await syntax continues to gain popularity, it is likely that this issue will affect more and more tsdx users.

Describe alternatives you've considered

With #183, tsdx users can hack tsdx's default plugin choices (example below) to either (1) to modify the rollup-plugin-typescript2 configuration as described above, or (2) swap out the plugin for rollup-plugin-ts; however, neither approach is ideal.

const ts = require("@wessberg/rollup-plugin-ts");

module.exports = {
  rollup(config, options) {
    config.plugins = config.plugins.map(plugin => {
      if (plugin && plugin.name === "rpt2") {
        return ts({
          tsconfig: tsconfig => ({
            ...tsconfig,
            target: "esnext",
            sourceMap: true,
            declaration: true,
            jsx: "react"
          }),
          transpiler: "babel"
        });
      }
      return plugin;
    });

   ...

    return config;
  }
};

For diagnostic purposes, a working example using the above swap-out hack can be found here: https://github.com/medelman17/tsdx-monorepo-template

Additional context

The rollup-plugin-typescript2 plugin appears to have several additional bugs that may cause tsdx users pain. For example, to enable code splitting, users must also employ an additional workaround, setting rollupCommonJSResolveHack to true. But see:

On windows typescript resolver favors POSIX path, while commonjs plugin (and maybe others?) uses native path as module id. This can result in namedExports being ignored if rollup happened to use typescript's resolution. Set to true to pass resolved module path through resolve() to match up with rollup-plugin-commonjs. rollup-plugin-commonjs fixed this in 10.1.0, so projects using this option who update to new version will be broken again. This also works around the similar bug affecting code splitting (see rollup/issues/3094).

@jaredpalmer
Copy link
Owner

jaredpalmer commented Sep 8, 2019 via email

@medelman17
Copy link
Contributor Author

medelman17 commented Sep 9, 2019 via email

@tunnckoCore
Copy link

tunnckoCore commented Sep 16, 2019

This switch will be amazing! :) I'm playing around in the last couple of days and the @wessberg plugin works beautifully. One huge benefit is that it bundles and tree-shakes the declarations files - no such thing possible until recently. And frees the road to use Babel for the rest. Which isn't exactly true, since when type-check tsc (and in your editor) will error. But at least that's until TypeScript 3.7.

I was working on a Rollup config that using it too. Some working example (with monorepo support) can be seen here (and the whole monorepo uses it too) https://github.com/tunnckoCore/configs/tree/master/packages/rollup-config. Usage is simple as

const config = require('@tunnckocore/rollup-config/config');

module.exports = config({ prod: true, monorepo: true }).concat(
  config({ prod: false, monorepo: true })
);
// generating cjs, esm, umd, 
// and index.d.ts, index.min.js, index.js.map files for each format

I'll check the #208

@medelman17
Copy link
Contributor Author

medelman17 commented Sep 16, 2019 via email

@agilgur5 agilgur5 added the topic: v0.10.x Issues related to broken builds in v0.10.x / @wessberg/rollup-plugin-ts label Mar 10, 2020
@agilgur5 agilgur5 changed the title Migrate Rollup TS Plugin to '@wessberg/rollup-plugin-ts' Migrate Rollup TS Plugin to '@wessberg/rollup-plugin-ts' to support async rollup plugins Mar 16, 2020
@agilgur5 agilgur5 added the topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 topic: v0.10.x Issues related to broken builds in v0.10.x / @wessberg/rollup-plugin-ts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants