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

Can't bundle clsx with rollup: clsx_1.default is not a function #27

Closed
scottc-netflix opened this issue Sep 23, 2020 · 4 comments · Fixed by #57
Closed

Can't bundle clsx with rollup: clsx_1.default is not a function #27

scottc-netflix opened this issue Sep 23, 2020 · 4 comments · Fixed by #57

Comments

@scottc-netflix
Copy link

scottc-netflix commented Sep 23, 2020

I think it might be similar to this issue I found: jednano/tsx-react-postcss-webpack#1

rollup 2.26.11
@rollup/plugin-node-resolve@9.0.0
@rollup/plugin-commonjs 15.0.0

Right before posting this I found a workaround, adding requireReturnsDefault to commonjs plugin

// in rollup.config.js
import { commonjs } from '@rollup/plugin-commonjs';

plugins: [
  commonjs({
    requireReturnsDefault: 'auto'
  })
]
@TrySound
Copy link

TrySound commented Sep 23, 2020

clsx provides es modules which should just work for rollup without commonjs plugin. Could you provide full config?
https://github.com/lukeed/clsx/blob/master/package.json#L6

@scottc-netflix
Copy link
Author

I think it's because I have a dependency that is commonjs and it's using clsx. It may be that rollup is just not smart enough to handle this situation without that setting. I thought it may be useful to someone else since clsx is a very commonly used library. Here is where they talk about that setting, which is actually fairly new in rollup: https://www.npmjs.com/package/@rollup/plugin-commonjs#requirereturnsdefault

If you remove that setting it will fail at runtime because it can't find the default export from clsx.

Here is my rollup config:

import path from 'path';
import ttypescript from 'ttypescript';  // 1.5.8
import babel from '@rollup/plugin-babel'; // 5.0.4
import commonjs from '@rollup/plugin-commonjs'; // 15.0.0
import nodeGlobals from 'rollup-plugin-node-globals'; // 1.4.0
import resolve from '@rollup/plugin-node-resolve';  // 9.0.0
import replace from 'rollup-plugin-replace';    // 2.2.0
import internal from 'rollup-plugin-internal';  // 1.0.4
import { terser } from 'rollup-plugin-terser';  // 5.1.1
import typescript from '@rollup/plugin-typescript';  // 2.0.1
import pkg from './package.json';

const isProduction = process.env.NODE_ENV === 'production' ? true : false;

const tsConfigPath = isProduction ? './tsconfig.prod.json' : './tsconfig.json';

const outputPath = './public';

const plugins = [
  babel({
    exclude: ['node_modules/**', 'src/**/*.ts', 'src/**/*.tsx'],
    babelHelpers: 'bundled'
  }),
  resolve({
    mainFields: ['module', 'main', 'browser'],
    preferBuiltins: false,
    jail: __dirname + '/node_modules',
    dedupe: ['react', 'react-dom']
  }),
  replace({
    PRODUCTION: JSON.stringify(isProduction)
  }),
  commonjs({
    sourceMap: isProduction ? false : 'inline',
    requireReturnsDefault: 'auto',
    include: [
      'node_modules/**',
    ],
  }),
  nodeGlobals({
    global: false,
    buffer: false,
    dirname: false,
    filename: false,
    baseDir: false
  }),
  (isProduction ? terser() : null),
  internal(['styled-components']),
  typescript({
    typescript: ttypescript,
    tsconfig: tsConfigPath,
    exclude: ['*.js', 'node_modules/**']
  })
];

const productionPath = process.env.BUILD === 'production' ? true : false;

export default [{
  input: 'src/index.tsx',
  output: [
    {
      dir: outputPath,
      entryFileNames: `${pkg.name}-[hash].es.js`,
      format: 'es',
      sourcemap: isProduction ? false : 'inline'
    },
  ],
  treeshake: true,
  watch: {
    exclude: ['node_modules/**'],
    chokidar: true,
    clearScreen: false
  },
  plugins: plugins
}];

@TrySound
Copy link

Try requireReturnsDefault: 'preferred'

@lukeed
Copy link
Owner

lukeed commented Sep 25, 2020

Yeah, this looks new from @rollup/plugin-commonjs@15.0 and is unintentionally a breaking change.

For clsx, and presumably any package that uses module.exports = ..., you have to use a non-default value for that new requireReturnsDefault option.

clsx can't migrate away from default exports because it's meant to be a drop-in replacement for classnames. Modifying exports would lose the ability for aliasing at the build and install levels.

Closing this as there's unfortunately nothing we can do on our end. It'd be a README call-out, at best.

Thank you @TrySound for helping out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants