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

react and csstips use module syntax rollup doesn't understand #933

Closed
kellycampbell opened this issue Jul 24, 2019 · 11 comments
Closed

react and csstips use module syntax rollup doesn't understand #933

kellycampbell opened this issue Jul 24, 2019 · 11 comments

Comments

@kellycampbell
Copy link
Contributor

🐞 bug report

Affected Rule

The issue is caused by the rule: rollup_bundle

Is this a regression?

Unknown

Description

In a rolled up js file, the browser has an error where a * import was used:

Uncaught TypeError: undefined is not a function

My original ts code:

import * as csstips from 'csstips';

console.log("Setting up page with csstips", csstips);
csstips.normalize();
csstips.setupPage('#root');

but the code where I used csstips has this in the es2015.js file:

console.log("Setting up page with csstips", csstips);
undefined();
undefined('#root');

My BUILD rules are like this:

ts_library(
    name = "app_lib",
    srcs = glob([
        "**/*.ts", 
        "**/*.tsx",
    ]),
    deps = [
        "@npm//csstips",
        "@npm//csx",
        "@npm//react",
        "@npm//react-dom",
        "@npm//tslib",
        "@npm//typestyle",
        "@npm//@types",
    ],
    tsconfig = "//:tsconfig.json",
)

rollup_bundle(
    name = "app_bundle",
    deps = [
        ":app_lib",
    ],
    entry_point = ":app.tsx",
    globals = {
        "react": "React",
        "react-dom": "ReactDOM",
    },
)
@kellycampbell
Copy link
Contributor Author

I found a working solution:

import csstips from 'csstips';

@surlyengineer
Copy link

Having a similar problem with import * as React from 'react', e.g.

@kellycampbell
Copy link
Contributor Author

I use import React from 'react';

My tsconfig has the following flags which create the __importStar and __importDefault:

        "esModuleInterop": true,
        "allowSyntheticDefaultImports": true,

@kellycampbell
Copy link
Contributor Author

On my original problem, it seems to be something specific to this csstips library, and I suspect also may be due to a bug in either rollup or typescript.

Another library from the same author as csstips works ok with the import *:

Works with rollup_bundle but doesn't work with ts_library:

import csstips from 'csstips';
import * as csx from 'csx';

Works with ts_library but not with rollup_bundle:

import * as csstips from 'csstips';
import * as csx from 'csx';

The result of all this is with React and using that csstips library I can do either server-side-rendering without hydration from the rollup bundle, or no SSR and including the bundle normally, but not both SSR + hydrate ("isomorphic").

@alexeagle
Copy link
Collaborator

alexeagle commented Aug 30, 2019

Seems like the --silent option we pass to rollup is causing it to hide legitimate errors.

Turning that off I see

Error: 'render' is not exported by 'bazel-out/k8-fastbuild/bin/bundle.es6/external/npm/node_modules/react-dom/index.js'
    at Object.onwarn (/home/alexeagle/.cache/bazel/_bazel_alexeagle/756357cd87a3a5c0e6a33f99a98529d8/execroot/examples_reactjs/bazel-out/k8-fastbuild/bin/_bundle.rollup.conf.js:176:11)
[!] Error: 'render' is not exported by 'bazel-out/k8-fastbuild/bin/bundle.es6/external/npm/node_modules/react-dom/index.js'
Error: 'render' is not exported by 'bazel-out/k8-fastbuild/bin/bundle.es6/external/npm/node_modules/react-dom/index.js'
    at Object.onwarn (/home/alexeagle/.cache/bazel/_bazel_alexeagle/756357cd87a3a5c0e6a33f99a98529d8/execroot/examples_reactjs/bazel-out/k8-fastbuild/bin/_bundle.rollup.conf.js:176:11)

Looking inside react-dom/index.js we see

if (process.env.NODE_ENV === 'production') {
  // DCE check should happen before ReactDOM bundle executes so that
  // DevTools can report bad minification during injection.
  checkDCE();
  module.exports = require('./cjs/react-dom.production.min.js');
} else {
  module.exports = require('./cjs/react-dom.development.js');
}

I think rollup doesn't know how to peek inside this if block to follow the re-exported symbols

alexeagle added a commit to codesuki/rules_nodejs that referenced this issue Aug 30, 2019
@alexeagle
Copy link
Collaborator

I see that the rollup-replace-plugin is meant to be used
rollup/rollup#487
I tried this in #1076 but not working yet...

alexeagle added a commit to codesuki/rules_nodejs that referenced this issue Aug 30, 2019
@alexeagle
Copy link
Collaborator

Not surprised csstips has this problem too
https://unpkg.com/csstips@1.1.0/lib/index.js

function __export(m) {
    for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
}
exports.__esModule = true;
/**
 * TODO: move out to csstips
 */
__export(require("./font"));

rollup doesn't know how to do the partial evaluation to follow the re-export structure there

@alexeagle
Copy link
Collaborator

rollup/rollup-plugin-node-resolve#58 describes our problem.
The rollup-plugin-commonjs doesn't understand that react's main entry point files export the symbols we use.

This snippet in the rollup.config.js makes the react example work:

commonjs({
      namedExports: {
        // left-hand side can be an absolute path, a path
        // relative to the current directory, or the name
        // of a module in node_modules
        'bazel-out/k8-fastbuild/bin/bundle.es6/external/npm/node_modules/react/index.js': ['Children', 'Component', 'PropTypes', 'createElement'],
        'bazel-out/k8-fastbuild/bin/bundle.es6/external/npm/node_modules/react-dom/index.js': ['render']
      }
    }),

I think we are being strange here by bundling react into the output bundle. Maybe most sites leave it as a separate script tag coming from a CDN?

alexeagle added a commit to codesuki/rules_nodejs that referenced this issue Sep 10, 2019
@alexeagle alexeagle changed the title rollup_bundle not working with Typescript importStar react and csstips use module syntax rollup doesn't understand Sep 23, 2019
@alexeagle
Copy link
Collaborator

Will probably close this once the new rollup_bundle is released, I don't see why this is bazel-specific.

@Toxicable
Copy link

Toxicable commented Sep 23, 2019

@alexeagle It would be still good to have an example rollup config on how to work with this though, right?

@alexeagle
Copy link
Collaborator

Doesn't look like this is a bazel problem, we should be running rollup the same way you would outside bazel.

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

No branches or pull requests

4 participants