calvinmetcalf / rollup-plugin-node-builtins

138 stars 40 forks source link

`os` doesn't work? #24

Closed unional closed 7 years ago

unional commented 7 years ago

I'm using node-globals and node-builtins to build my bundle.

In the code, I use import { release } from 'os':

https://github.com/unional/aurelia-logging-color/blob/rollup-require-issue/src/environments.ts

When I bundle and load the script on the browser, it complains:

image

You can see the error by cloning the repo:

git checkout rollup-require-issue
npm install
npm run build
// or 'start' if you are using windows
open demo.html

When I use webpack, it is working correctly.

Here is the rollup configuration: https://github.com/unional/aurelia-logging-color/blob/rollup-require-issue/rollup.config.global.js

import paramCase from 'param-case';
import pascalCase from 'pascal-case';
import commonjs from 'rollup-plugin-commonjs';
import nodeBuiltins from 'rollup-plugin-node-builtins';
import nodeGlobals from 'rollup-plugin-node-globals';
import nodeResolve from 'rollup-plugin-node-resolve';
import sourcemaps from 'rollup-plugin-sourcemaps';
import uglify from 'rollup-plugin-uglify';

const pkg = require('./package');

const moduleName = pascalCase(pkg.name)

export default {
  dest: `dist/${paramCase(pkg.name)}.js`,
  entry: 'dist/commonjs/index.js',
  exports: 'named',
  external: [
    'color-map'
  ],
  format: 'iife',
  globals: {
    'color-map': 'ColorMap'
  },
  moduleId: pkg.name,
  moduleName,
  plugins: [
    sourcemaps(),
    nodeResolve({
      jsnext: true,
      skip: ['color-map']
    }),
    nodeGlobals(),
    nodeBuiltins(),
    commonjs(),
    uglify()
  ],
  sourceMap: true
};

Here is the webpack configuration:

https://github.com/unional/aurelia-logging-color/blob/rollup-require-issue/webpack.config.global.js

'use strict';
const paramCase = require('param-case')
const pascalCase = require('pascal-case')
const path = require('path')
const webpack = require('webpack')

const pjson = require('./package.json')

const packageName = pjson.name
const version = pjson.version
const filename = paramCase(packageName)
const globalVariable = pascalCase(filename)

module.exports = {
  externals: {
    "color-map": "ColorMap"
  },
  devtool: 'source-map',
  entry: {
    [filename]: './dist/commonjs/index'
  },
  output: {
    path: path.join(__dirname, 'dist'),
    filename: `${filename}.js`,
    library: globalVariable,
    libraryTarget: 'var'
  },
  module: {
    rules: [
      {
        enforce: 'pre',
        loader: "source-map-loader",
        test: /\.js?$/
      }
    ]
  },
  plugins: [
    new webpack.BannerPlugin(`${filename}.js version: ${version} generated on: ${new Date().toDateString()}`)
  ]
}

Thanks. 🌷

UPDATE: pointing to rollup-require-issue branch., add repro steps

calvinmetcalf commented 7 years ago

your type-script build turns your imports into requires so this is totally not related to this particular plugin but more generally combining typescript and rollup

unional commented 7 years ago

It seems like this is more related to rollup-plugin-commonjs than to rollup-plugin-node-builtins.

Thanks.

I ran this is transpiled directly by tsc and the transpilation target is es5. Does rollup-plugin-commonjs and rollup-plugin-node-builtins support es5 or only es6/es2015?

I want to bundle the es5 version because my (personal) use cases involve supporting older browsers.

calvinmetcalf commented 7 years ago

rollup is designed for bundling es6 modules rollup-plugin-commonjs is for use if you have some commonjs dependencies you need to use as well, if everything is es5 there isn't any point in using rollup, other things work better.

the problem you have is tsc is transpiling the syntax AND the import statements into es5, you want the syntax but not the import statements transpiled and I belive you can use the typescript plugin to do so