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

Breaks dependency treeshaking? -- node-resolve and commonjs plugins should be first #87

Closed
dnbkr opened this issue Jun 8, 2018 · 3 comments
Labels
kind: support Asking for support with something or a specific use case problem: plugin order The plugin order in this issue seems incompatible. See the "Compatibility" section in the README scope: integration Related to an integration, not necessarily to core (but could influence core)

Comments

@dnbkr
Copy link

dnbkr commented Jun 8, 2018

What happens and why it is wrong

Hey

I'm new to rollup, and pretty new to typescript, but I'm evaluating both for use in a react component library with a focus on small bundle size. Just starting out really. Anyway - when writing my components in JS, rollup will tree shake dependencies and only bundle the functions in use. When I add the typescript plugin, this isn't done.

Steps to recreate:

In one of the demo components, the function lifecycle is used from the package recompose. Nothing else from recompose is used.

and you will see the function withState from recompose is included in the build. Now comment out the typescript plugin from rollup.config.js and re run the build and grep and you'll see that withState is not bundled, though grep for lifecycle and that has been included correctly.

Am I doing it wrong? Not sure if this is my issue or something wrong with the plugin.

Environment

OS: Mac OS 10.13.4
Node: 8.11.1

Versions

  • typescript: 2.9.1
  • rollup: 0.60.1
  • rollup-plugin-typescript2: 0.14.0

rollup.config.js

import babel from 'rollup-plugin-babel'
import commonjs from 'rollup-plugin-commonjs'
import external from 'rollup-plugin-peer-deps-external'
import postcss from 'rollup-plugin-postcss'
import resolve from 'rollup-plugin-node-resolve'
import url from 'rollup-plugin-url'
import typescript from 'rollup-plugin-typescript2'

import pkg from './package.json'

export default {
  input: 'src/index.js',
  output: [
    {
      file: pkg.module,
      format: 'es'
    }
  ],
  plugins: [
    external(),
    postcss({
      modules: true
    }),
    url(),
    typescript({
      verbosity: 3
    }),
    babel({
      exclude: 'node_modules/**'
    }),
    resolve(),
    commonjs()
  ]
}

tsconfig.json

{
  "compilerOptions": {
    "jsx": "React"
  }
}

package.json

{
  "name": "rollup-test",
  "version": "1.0.0",
  "module": "dist/index.js",
  "scripts": {
    "build": "rollup -c"
  },
  "peerDependencies": {
    "prop-types": "^15.6.1",
    "react": "^16.4.0"
  },
  "dependencies": {
    "classnames": "^2.2.6",
    "recompose": "^0.27.1"
  },
  "devDependencies": {
    "@types/react": "^16.3.17",
    "babel-core": "^6.26.3",
    "babel-plugin-external-helpers": "^6.22.0",
    "babel-preset-env": "^1.7.0",
    "babel-preset-react": "^6.24.1",
    "rollup": "^0.60.1",
    "rollup-plugin-babel": "^3.0.4",
    "rollup-plugin-commonjs": "^9.1.3",
    "rollup-plugin-node-resolve": "^3.3.0",
    "rollup-plugin-peer-deps-external": "^2.1.1",
    "rollup-plugin-postcss": "^1.6.2",
    "rollup-plugin-typescript2": "^0.14.0",
    "rollup-plugin-url": "^1.4.0",
    "typescript": "^2.9.1"
  }
}

plugin output with verbosity 3

log.txt

@ezolenko
Copy link
Owner

ezolenko commented Jun 8, 2018

Don't know which part exactly breaks it, but to fix it you need to do several things -- move resolve and common js plugins before ts plugin, modify tsconfig to allow js and include only src dir, and rename js files into jsx.

diff --git a/rollup.config.js b/rollup.config.js
index 1dfd42b..c26173c 100644
--- a/rollup.config.js
+++ b/rollup.config.js
@@ -22,13 +22,14 @@ export default {
       modules: true
     }),
     url(),
+    resolve(),
+    commonjs(),
     typescript({
-      verbosity: 3
+      verbosity: 2,
+      include: ["*.js|*.jsx"]
     }),
     babel({
       exclude: 'node_modules/**'
-    }),
-    resolve(),
-    commonjs()
+    })
   ]
 }
diff --git a/src/button/index.js b/src/button/index.jsx
similarity index 100%
rename from src/button/index.js
rename to src/button/index.jsx
diff --git a/src/link/index.js b/src/link/index.jsx
similarity index 100%
rename from src/link/index.js
rename to src/link/index.jsx
diff --git a/tsconfig.json b/tsconfig.json
index 698b574..218afb8 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,5 +1,9 @@
 {
   "compilerOptions": {
-    "jsx": "React"
-  }
+    "jsx": "React",
+    "allowJs": true
+  },
+  "include": [
+    "src"
+  ]
 }
\ No newline at end of file

@dnbkr
Copy link
Author

dnbkr commented Jun 8, 2018

Just moving the plugin invocation to the bottom of the array seemed to do the trick - can I ask what the other changes you mention achieve? I notice that when I apply them I can no longer export ts declarations from the components that are written in ts (note, I want to support both ts and js components)

@dnbkr dnbkr closed this as completed Jun 8, 2018
@ezolenko
Copy link
Owner

ezolenko commented Jun 8, 2018

  • "allowJs": true was to let typescript even see js files -- it ignores them otherwise, so the plugin wasn't doing much at all. You want that if you want ts to do the checks on js. Other changes come out of that.
  • Renaming to jsx was because ts didn't like HTML inside js otherwise
  • "include": [ "src ] was to make sure all other folders were excluded, otherwise ts wanted to process dist and stuff.

@agilgur5 agilgur5 added kind: support Asking for support with something or a specific use case problem: plugin order The plugin order in this issue seems incompatible. See the "Compatibility" section in the README labels May 22, 2022
@agilgur5 agilgur5 changed the title Plugin breaks Treeshaking? Breaks dependency treeshaking? -- wrong plugin order, node-resolve and commonjs plugins should be first May 22, 2022
@agilgur5 agilgur5 added the scope: integration Related to an integration, not necessarily to core (but could influence core) label May 22, 2022
Repository owner locked as resolved and limited conversation to collaborators May 22, 2022
@agilgur5 agilgur5 changed the title Breaks dependency treeshaking? -- wrong plugin order, node-resolve and commonjs plugins should be first Breaks dependency treeshaking? -- node-resolve and commonjs plugins should be first May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: support Asking for support with something or a specific use case problem: plugin order The plugin order in this issue seems incompatible. See the "Compatibility" section in the README scope: integration Related to an integration, not necessarily to core (but could influence core)
Projects
None yet
Development

No branches or pull requests

3 participants