d3 / d3-request

A convenient alternative to XMLHttpRequest.
BSD 3-Clause "New" or "Revised" License
110 stars 54 forks source link

using rollup with `browser: true` interferes with es6 imports #21

Closed micahscopes closed 7 years ago

micahscopes commented 7 years ago

with d3-selection, I can do: import { select } from 'd3-selection'

But I'm having trouble doing the same kind of thing with d3-request. I'd like to be able to do somethin like: import { json } from 'd3-request'

what am I missing?

mbostock commented 7 years ago

That should work. However, you haven’t described your build environment (the contents of your package.json, what build tool you are using, etc.), or the exact trouble (such as a stack trace) you are experiencing, so I can’t say why it’s not working for you.

Please use Stack Overflow tag d3.js to ask for help. Although I make an effort to assist everyone that asks, I am not always available to provide help promptly or directly. Stack Overflow provides a better collaborative forum for self-help: tens of thousands of D3-related questions have already been asked there, and some answered questions may be relevant to you.

When asking for help, please include a link to a live example that demonstrates the issue, preferably on bl.ocks.org. It is often impossible to debug from code snippets alone. Isolate the issue and reduce your code as much as possible before asking for help. The less code you post, the easier it is for someone to debug, and the more likely you are to get a helpful response.

If you have a question about D3’s behavior and want to discuss it with other users, also consider the d3-js Google Group or joining the d3-js Slack.

Thank you! 🤗

micahscopes commented 7 years ago

I'll do some digging and get back to you.

micahscopes commented 7 years ago

By the way, thanks for all you do! You're right up there with Ton Roosendaal in my book :D

mbostock commented 7 years ago

You’re welcome. If you give me enough details to investigate I will be happy to take a look.

curran commented 7 years ago

@micahscopes Maybe fork and modify one of these?

You can pull down either one by cloning the Gist as a Git repository. Then you can post your modified version as a new Gist by deleting the .git directory and running gistup. Good luck!

micahscopes commented 7 years ago

Thanks @curran for the tip, that really saved me some time!

@mbostock I have isolated the issue here (changelog): https://gist.github.com/micahscopes/0481ea69ab0d9310458250fb70bf35e4

If you run rollup -c rollup.config.js, you will likely get:

'json' is not exported by '/.../node_modules/d3-request/build/d3-request.js' (imported by '/.../index.js')

The problem is related to my using jsnext: true alongside browser: true in rollup-plugin-node-resolve.

I was using browser: true naively, since my target is the browser. But idk if it really matters. I love that I can get such small bundles with rollup, but it sure can be confusing! Today I just about gave up on roughly four libraries that I wanted to use (including two of my own) because their es6 support was so dire! :)

micahscopes commented 7 years ago

Seems like this is related to https://github.com/rollup/rollup-plugin-node-resolve/issues/8

curran commented 7 years ago

@micahscopes Having battled with Rollup myself to get things to work, I can totally relate.

Here's a Rollup config that covers all the cases I needed for one of my projects:

import npm from "rollup-plugin-node-resolve";
import babel from "rollup-plugin-babel";
import replace from "rollup-plugin-replace";
import commonjs from "rollup-plugin-commonjs";

export default {
  entry: "src/client/index.js",
  format: "iife",
  plugins: [
    npm({
      jsnext: true
    }),
    commonjs(),
    babel({
      exclude: "node_modules/**"
    }),
    replace({ // Fixes build error with Redux.
      "process.env.NODE_ENV": "'production'"
    })
  ],
  dest: "example-viewer-dist/bundle.js"
};

Some points to note:

mbostock commented 7 years ago

@micahscopes A few problems.

  1. You are using old software. You should be using rollup@41 and rollup-plugin-node-resolve@2.

  2. You don’t have d3-request listed as a (dev-)dependency in your package.json.

Here is a fixed example; see the last revision for my changes:

https://gist.github.com/mbostock/4814f31eb9a01d9f610b88b6ff42e91d

micahscopes commented 7 years ago

@mbostock I think I forgot to commit the package.json to my fork! wOops! :blush:

Ok, so forgive my being pedantic... but I figure it's worth it to document what's going on.

I forked your updated gist and am still experiencing the same issue when I use browser: true. I think the real issue here is a rollup issue, how the browser flag interacts with module, jsnext, etc. (as discussed here). Right now, it seems like the browser flag takes precedent over module or jsnext:main, and in the case of d3-request, it's trying to find the json export in a file made for the browser, so it spits out:

'json' is not exported by node_modules/d3-request/build/d3-request.js

But I've realized it's not necessary in my case to use that browser flag... so that's my solution for, for now!

I have forked here: https://gist.github.com/micahscopes/44479681bb2354ccbf6b822e244bb0a4

The key change being in rollup.config.js:

import node from "rollup-plugin-node-resolve";

export default {
  plugins: [node({
    browser: true
  })]
};