folktale / data.task

Migrating to https://github.com/origamitower/folktale
MIT License
425 stars 45 forks source link

Remove references to process #39

Closed bagrounds closed 7 years ago

bagrounds commented 7 years ago

When trying to browserify a module that depends on data.task, I get the following error:

Cannot find module '_process' from '/home/.../node_modules/data.task/lib'

Browserify seems to be trying to require process because of this line: https://github.com/folktale/data.task/blob/master/lib/task.js#L10

If I remove the references to process, from the source code, browserify works.

What's the point of using process.nextTick anyway? Seems like some kind of optimization that may not be necessary.

Can we remove it?

Thanks!

robotlolita commented 7 years ago

delayed is used to schedule cleanup routines. Since we also support older Node platforms, where setImmediate isn't available, that leaves us with process.nextTick.

That said, Browserify should handle this correctly, by loading https://github.com/defunctzombie/node-process automatically. It looks like that's not happening (maybe because of some configuration?). Do you have a minimal test case that reproduces this problem?

bagrounds commented 7 years ago

@robotlolita - thanks for the quick reply, and for explaining the purpose of process.nextTick.

tl;dr

I no longer think data.task is the cause of the problem I'm facing - the problem is more likely that I'm doing something wrong, or I found a weird edge case in browserify.

long story

In my search for a minimal test case, I found that the issue is related to the --no-browser-field (--no-bf) flag in browserify.

Minimal test case:

index.js

require('data.task')

package.json

{
  "main": "index.js",
  "scripts": {
    "browserify": "browserify index.js > bundle.js"
  },
  "dependencies": {
    "browserify": "^14.3.0",
    "data.task": "^3.1.1"
  }
}
$ npm run browserify # works fine

Motivation for the following change

I use unpkg.com to serve my browserified modules. The way I'm doing this currently is to make a dist folder with an entry.js file containing the single line moduleName = require('../src/index.js') (I typically keep my main file at src/index.js). Then I do

$ browserify dist/entry.js > dist/bundle.js

This lets me include my browserified version in a browser with a script tag pointing at unpkg.com/moduleName - but I have to set the browser field of package.json to point at dist/bundle.js.

Continuing with the simplified example

If I change package.json to this:

{
  "main": "index.js",
  "browser": "bundle.js",
  "scripts": {
    "browserify": "browserify index.js > bundle.js"
  },
  "dependencies": {
    "browserify": "^14.3.0",
    "data.task": "^3.1.1"
  }
}

When I run

$ npm run browserify

I don't get an error, but the resulting bundle.js is not correct. I get something like this:

(function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){

},{}]},{},[1]);

So I go scour the browserify documentation and find the --no-bf flag. Using this flag fixes this problem in other modules that do not depend on data.task (for example: https://gitlab.com/bagrounds/fun-array/blob/master/package.json#L20), but I get an error if I'm depending on data.task.

So with a package.json like this:

{
  "main": "index.js",
  "browser": "bundle.js",
  "scripts": {
    "browserify": "browserify --no-bf index.js > bundle.js"
  },
  "dependencies": {
    "browserify": "^14.3.0",
    "data.task": "^3.1.1"
  }
}

I get the following error:

$ npm run browserify
Error: Cannot find module '_process' from './node_modules/data.task/lib'
robotlolita commented 7 years ago

Thanks for the very detailed report :)

You can get your bundle working by either using a different entry point than the one defined as main in your package.json for Browserify, or by defining your bundle as unpkg instead of browser in the package.json:

{
  "main": "index.js",
  "unpkg": "bundle.js",
  "scripts": {
    "browserify": "browserify index.js > bundle.js"
  },
  "dependencies": {
    "browserify": "^14.3.0",
    "data.task": "^3.1.1"
  }
}

From what I understand, the problem is that you're defining the browser entry point as your bundle, and main as index.js. Because Browserify will try to resolve things with the browser file when one is available, browserify index.js > bundle.js will be interpreted as:

browserify bundle.js > bundle.js

Since index.js is the file defined in main.js, and a browser alternative is defined for that file (see https://github.com/defunctzombie/package-browser-field-spec for details). This will result in the empty file you're seeing.

If you disable the browser field resolution (with --no-bf), Browserify will switch to a more Node-oriented mode. I think the special variable resolution should still work here, but I'm not familiar with the goals/code of Browserify to say if this is a bug or not, you might want to open an issue on their issue tracker on this. It does mean that things will probably not work if you're requiring a module that's been written for both Node and Browsers and needs to load different code in each platform.

You could disable Browserify's analysis of these variables here, like:

browserify --no-bf --insert-global-vars __dirname,__filename index.js > bundle.js

And it would work with data.task (because it checks if process is a global before using it), but might not work with other modules. Buffer and global would also not be inserted.