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

Output tree-shakeable friendly classes when compiling ES5 #182

Closed
k2snowman69 opened this issue Oct 13, 2019 · 6 comments
Closed

Output tree-shakeable friendly classes when compiling ES5 #182

k2snowman69 opened this issue Oct 13, 2019 · 6 comments
Labels
controversial kind: feature New feature or request kind: optimization Performance, space, size, etc improvement solution: out-of-scope This is out of scope for this project solution: tsc behavior This is tsc's behavior as well, so this is not a bug with this plugin solution: wontfix This will not be worked on

Comments

@k2snowman69
Copy link

k2snowman69 commented Oct 13, 2019

What happens and why it is wrong

I've been investigating why one of my React Typescript component libraries doesn't treeshake nicely. An important fact is that I'm outputting ES5 target.

The cause I've come down to is because terser works best with es2015 modules and defaults to a more safe output. Since ES5 doesn't have import/export type situations it's hard terser to know what is safe to strip out. However with typescript I think there is a solution and I ran into it here specifically this custom plugin:

    plugins: [
        resolve({ jsnext: true, module: true }),
        {
            transform(code, id) {
                return code.replace(/\/\*\* @class \*\//g, "\/*@__PURE__*\/");
            }
        },
        uglify(uglify_options),
    ],

When typescript compiles, it adds a /** @class */ comment in front of every class object it transpiles. Simply replacing this class comment with Terser's __PURE__ comment allows Terser to know it can tree shake these objects.

I further checked to make sure if prop-types is being used it wont remove the function. For example the following would be tree-shaked

export class Example1Component extends React.Component {
  render() {
    return 'hello1';
  }
}

but the following would not due to it having possible unintended consequences

export class Example1Component extends React.Component {
  render() {
    return 'hello1';
  }
}
Example1Component.defaultValues = {
  wheee: 123,
};

So the ask here is that rollup-plugin-typescript2 does the replacement of /** @class */ to /*@__PURE__*/ so that the ES5 outputs would be tree-shaken down stream.

Environment

N/A

Versions

  • typescript: 3.6.4
  • rollup: 1.23.1
  • rollup-plugin-typescript2: 0.24.3

rollup.config.js

import pkg from './package.json';
import typescript from 'rollup-plugin-typescript2';

export default {
  external: [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.peerDependencies || {})],
  input: 'src/index.ts',
  output: [
    {
      file: pkg.main,
      format: 'cjs',
    },
    {
      file: pkg.module,
      format: 'es',
    },
  ],
  plugins: [
    typescript({ tsconfig: 'tsconfig.json' }),
  ],
};

tsconfig.json

{
  "compilerOptions": {
    "declaration": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "lib": ["es5"],
    "module": "es2015",
    "moduleResolution": "node",
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": false,
    "outDir": "./dist/",
    "sourceMap": true,
    "strictNullChecks": true,
    "target": "es5"
  },
  "exclude": ["./src/**/*.test.ts", "./src/**/*.test.tsx", "./test/**/*.ts", "./test/**/*.tsx"],
  "include": ["./src/**/*.ts", "./src/**/*.tsx", "src/index.ts"]
}

package.json

Only relevant part is my build script

rollup -c rollup.config.js

plugin output with verbosity 3

N/A

@k2snowman69 k2snowman69 changed the title Output terser friendly classes Output terser friendly classes when compiling ES5 Oct 13, 2019
@k2snowman69 k2snowman69 changed the title Output terser friendly classes when compiling ES5 Output tree-shakeable friendly classes when compiling ES5 Oct 13, 2019
@k2snowman69
Copy link
Author

k2snowman69 commented Oct 13, 2019

It's also possible that the fix for this is in the rollup-plugin-terser side or on terser to consider /** @class */ a shakeable object similar to what uglify fixed a while ago

@ezolenko
Copy link
Owner

This looks like something a separate plugin should do, either rollup plugin like you had, or a typescript transformer. I'd rather not modify typescript output for everybody, too many possible unintended consequences.

@ezolenko ezolenko added controversial kind: feature New feature or request labels Oct 15, 2019
@k2snowman69
Copy link
Author

That makes sense, let me follow up with the terser repository as it may have broader impacts than just rollup

@k2snowman69
Copy link
Author

I'm going to close this, seems most suggestions is to use a transformer as I already was. Looks like babel also has a transformer to add the PURE script into it's ES5 output. Sorry for the noise!

@ccrowley96
Copy link

Any update on how you got around this @k2snowman69 ?

@k2snowman69
Copy link
Author

k2snowman69 commented Sep 9, 2020

Sigh, I hate replying when I know people are going to thumbs down it. Here are the changes I made to get this to work:

  1. Moved all dev-dependencies that I was including in my bundle into dependencies and let the consuming project deal with the bundling
  2. In the package.json mark the project as side-effect free - https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free
  3. Remove all of rollup from my build and just use tsc. Most importantly, make sure each module you want to have be tree-shakeable is a separate dist file build output. When they are all one file, it's hard for the tree shaker to split apart. As separate files, it's much easier on the tree-shaking algorithm. For me, I moved from using out to outDir.
  4. Rely on the consuming app to add minification on their final output which would get applied to my libraries output as well

Again, not the answer a rollup repository is going to want to hear, however it did remove a ton of dependencies and complications for me which actually made maintenance easier.

@agilgur5 agilgur5 added solution: out-of-scope This is out of scope for this project kind: optimization Performance, space, size, etc improvement solution: wontfix This will not be worked on solution: tsc behavior This is tsc's behavior as well, so this is not a bug with this plugin labels May 7, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial kind: feature New feature or request kind: optimization Performance, space, size, etc improvement solution: out-of-scope This is out of scope for this project solution: tsc behavior This is tsc's behavior as well, so this is not a bug with this plugin solution: wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants