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

Respect browser field in package.json? #1574

Closed
matthewwithanm opened this issue May 4, 2015 · 16 comments
Closed

Respect browser field in package.json? #1574

matthewwithanm opened this issue May 4, 2015 · 16 comments

Comments

@matthewwithanm
Copy link

I'm a library maintainer with a package that supports both node and the browser, using a browser field in package.json. Recently, an issue was raised about the library not working correctly in Electron because, even though the code is being run by Chromium (the client), Electron doesn't respect the browser field when requiring modules.

I realize that this isn't exactly a normal browser, but it seems to me that it would be logical for the client to use browser versions of modules when they exist. Thoughts?

@zcbenz
Copy link
Member

zcbenz commented May 5, 2015

There are some significant differences between node's require and things like node-browserify, modules targeted for browser might not work well in Electron. And Electron uses io.js for all node stuff, having different behaviors when loading modules doesn't sound like a good idea to me, so I'm closing this as won't fix.

@anaisbetts
Copy link
Contributor

I realize that this isn't exactly a normal browser, but it seems to me that it would be logical for the client to use browser versions of modules when they exist.

You're generally right though, while it's fine to use Electron's require, you generally want to use the DOM API if you've got it, since they often handle more edge cases than the node version (for example, if you use XHR instead of node's request module, you get proxy support for free instead of trying to do it yourself)

@matthewwithanm
Copy link
Author

Yeah, that was my reasoning. As it stands, most isomorphic libraries will need to be updated to do browser feature detection in their node implementations.

@yoshuawuyts
Copy link
Contributor

This little gotcha has cost me a good chunk of my time in the past week. electron ignoring the browser field creates some very surprising behavior.

debug

debug checks the browser field, and if not present will route all output to the terminal. Logging DOM elements or deeply nested objects becomes no longer practical, providing a worsened debugging experience.

nets

nets checks the browser field to determine if it should use xhr or request (which share an api). In electron it will run request, leaving behind a blank network tab. This makes it impossible to share components between regular web apps and electron apps that rely on nets.


I'm sure there are many more modules affected by this both directly and indirectly. Would you consider respecting the browser field? I think there's much to gain and less to lose.

@0x80
Copy link

0x80 commented Jul 6, 2015

+1 please reopen/consider

@anaisbetts
Copy link
Contributor

To be honest, these libraries need to just special-case Electron, a browser field is almost certainly going to get it wrong and you'll have to go around patching these libraries anyways. I get that you really want 'browser' to Just Work, but it almost certainly won't. As @zcbenz says, Electron is kind of neither a node.js nor a Chrome, what you want depends on what you're doing.

@yoshuawuyts
Copy link
Contributor

@paulcbetts could you explain a little more what those libraries would get wrong? I'm still not clearly seeing why the browser field wouldn't work.

Also: how would libraries be able to special case themselves to resolve correctly in electron? I'm down to write PRs for those libraries specifically if I know how to get them to behave. Thanks!

@anaisbetts
Copy link
Contributor

@yoshuawuyts So for example, many libraries which would be running under 'browser' would (quite reasonably) assume that they are running under node-browserify - in Electron, require has node.js Zen Nature (i.e. it's synchronous). Or for example, jQuery sees the presence of require and assumes that it shouldn't put itself onto window.$ when in Electron it probably should.

It isn't a class of issues, it's a million one-off bugs that won't be resolved by respecting browser. And in fact, libraries that are already Electron-aware will now break because this will be a significant change in behavior.

@yoshuawuyts
Copy link
Contributor

Ah, alright. That makes sense. Thanks!

@peter-mw
Copy link

@paulcbetts What about embedded apps, that are using <iframe> and deppend on browser to be present? How they can work in this case?

@techninja
Copy link

Maybe we should agree on a node package.json hybrid field referenced during require that is used by only by browser/node applications? Hrmm :/

@mattdesl
Copy link
Contributor

mattdesl commented Nov 1, 2015

How about just an option to respect browser field? browserField: true

Or a function to let the user handle it and give them full control on a per-module basis:

var resolve = require('browser-resolve')
new BrowserWindow({
  resolve: resolve
})

There are many instances where the "browser" field is a better choice for an Electron app. Currently the only way to get around this is to use Browserify or Webpack, which adds a lot of complexity to the development/build process.

@yoshuawuyts
Copy link
Contributor

Having it as an option would probably be the proper resolution. I've tried working around this, and it's been painful.

@mattdesl
Copy link
Contributor

Here is how I've added this feature to devtool.

The hook needs to be set before you require your entry. Once it goes through some more testing I may release it as a standalone module.
https://github.com/Jam3/devtool/blob/7c4e118ea16b13a2eab732762d1b01f5b76b66f2/lib/require-hook.js#L67-L79

@yoshuawuyts
Copy link
Contributor

@mattdesl that seems like a proper solution; that way electron can keep its defaults like it wants, and users can opt into different behavior if they like 👌

imlucas added a commit to mongodb-js/compass that referenced this issue Mar 16, 2016
@paulcbetts is haunting me…
debug-js/debug#206 (comment)

See also very relevant and interesting workarounds
electron/electron#1574
@yoshuawuyts
Copy link
Contributor

I've got a lil pattern for hybrid modules that need to run in Electron:

var isElectron = require('is-electron')
var browser = require('./browser')

if (typeof window !== 'undefined' && isElectron()) {
  module.exports = browser
} else {
  module.exports = myServerFunction
}

This makes it so the renderer process and browsers resolve to browser.js, and electron's main process and node resolve to index.js. Yay!

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

No branches or pull requests

9 participants