Object form of browser in package.json not supported

[IstoraMandiri]

_1 Upvote_ Meteor 1.3.2.4

There appears to be some broken behaviour when installing npm packages that use a browser field in package.json to specify browserify-compatible code.

package.json in question:

  "dependencies": {
    "browserify-sha3": "^0.0.1",
    "sha3": "^1.1.0"
  },
  "browser": {
    "sha3": "browserify-sha3"
  }

Report copied from: axic/keccakjs#1:

Seems like Meteor 1.3 ES6 import is having issues with this package, and is affecting other packages depending on keccakjs.

Reproduction:

meteor create keccak-test
cd keccak-test
meteor npm install
meteor npm install --save keccakjs

client/main.js

import keccakjs from 'keccakjs'

(same outcome with require)

---

Errors:
node console

Unable to resolve some modules:

  "build/Release/sha3" in /Users/chris/code/test/keccakjs-meteor-test/node_modules/keccakjs/node_modules/sha3/package.json (web.browser)

If you notice problems related to these missing modules, consider running:
  meteor npm install --save build
// running the above has no effect

browser console

Uncaught Error: Cannot find module 'sha3'
require @ install.js:78
meteorInstall.node_modules.keccakjs.index.js @ index.js:1
fileEvaluate @ install.js:141
require @ install.js:75
meteorInstall.client.main.js @ main.js:1
fileEvaluate @ install.js:141
require @ install.js:75
(anonymous function) @ app.js?hash=269ed68…:47

---

Related:

• Meteor 1.3 Integration Consensys/eth-lightwallet#87
• https://forums.meteor.com/t/meteor-1-3-browserify-potential-issue/20155
• (Maybe related?) https://gist.github.com/defunctzombie/4339901

---

Seems like it's not respecting the browser field in package.json. Hard-coding the browserified package 'fixes' the problem.

module.exports = require('browserify-sha3').SHA3Hash

[benjamn]

We do respect the "browser" field, but only when its value is a single string rather than an object containing multiple mappings, since the string case is much more common. See here for the relevant code.

Another very common use of the "browser" field is to provide stubs for Node's built-in modules like fs and http. We provide those stubs if you npm install meteor-node-stubs, which helps reduce the need for object-valued "browser" fields.

I'm open to implementing full support for the "browser" field, though I will also mention two other possible solutions:

• If the sha3 package specified its own browser-appropriate main module via the "browser" field of its package.json file, then keccakjs wouldn't have to make that decision on behalf of sha3.
• If keccakjs called require("browserify-sha3") in its client code rather than expecting require("sha3") to work as-is on the client, then it wouldn't have to rely on a magical property of package.json to disambiguate the two, and it would be clearer that those modules are actually not the same.

Supporting non-standard properties of package.json files could be a good idea if it solves a common problem for many people, though it can also lead down a slippery slope, since there are no restrictions on properties allowed in package.json, and it's up to your tools to decide what a custom property means.

[IstoraMandiri]

Keccakjs maintainer has modified to work with Meteor. Thanks for the help both!

[feross]

WebTorrent uses the object version of the "browser" field, so this is broken in Meteor right now.

See: https://github.com/feross/webtorrent/blob/master/package.json#L10-L19

[LazerJesus]

+1

[vvo]

Would be nice yes to implement the full browser field like webpack and browserify does: https://github.com/defunctzombie/package-browser-field-spec

[rosscreighton]

jsdom also uses the object version of the "browser" field

[sakulstra]

Superagent https://github.com/visionmedia/superagent/blob/master/package.json#L53 and spotify-web-api-node https://github.com/thelinmichael/spotify-web-api-node/blob/master/package.json#L50 use the object version of browser field and are broken :/

[Mediref]

+1 to get this fixed :)

[ilya-pirogov]

AWS SDK is broken too. https://github.com/aws/aws-sdk-js/blob/master/package.json#L49-L54

[damonmaria]

@ilya-pirogov I managed to get this working by first: import 'aws-sdk/lib/browser_loader'

Then in my case working with S3 wasn't a problem when imported this way: import S3 from 'aws-sdk/clients/s3'

If you need to access the global AWS object then import it as: import AWS from 'aws-sdk/browser'

I'm not sure this will work in all circumstances, but it worked for me so far.

[ilya-pirogov]

@damonmaria Thank you! It really works. Tested with DynamoDB and SQS.

[javoire]

Another one here :) https://github.com/mapbox/mapbox-gl-js/blob/master/package.json#L99, meteor bundles the file meant for node, and not the one meant for the browser, which breaks my app. Cheers!

[HemalR]

+1 for a fix. Affecting a lot of useful services:

1. Algolia Search
2. Mailguns client side email validation
3. Keen.io's client side data vis libraries

Does anyone have a work-around I can use in the meantime?

[vangorra]

For portability, we are using graphql for backed communication and avoided using meteor specific calls. The best work around we found was to ditch meteor all together. It made things much easier in the long run and allowed us to use serverless architecture.
To transition, we built our own client code and used meteor for the backed only. After a bit of that, porting to lambda was pretty straight forward.

[HemalR]

Hmmm.. I know Meteor well enough that it would kill speed for now. But I think I am absolutely going to push out a prototype/MVP and do something similar... Cheers for the input!

…

On Thu, Sep 21, 2017 at 5:05 PM, Robert Van Gorkom ***@***.*** > wrote: For portability, we are using graphql for backed communication and avoided using meteor specific calls. The best work around we found was to ditch meteor all together. It made things much easier in the long run and allowed us to use serverless architecture. To transition, we built our own client code and used meteor for the backed only. After a bit of that, porting to lambda was pretty straight forward. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6890 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AO6Pf5xQ5MooXv6dYnecD-VZZz5zH5xgks5skibOgaJpZM4IOcAR> .

[aldeed]

I ran into this trying out @firebase/firestore pkg. Here are repro instructions if it helps:

1. Install Meteor: curl https://install.meteor.com/ | sh
2. meteor create --bare firestore-issue
3. cd firestore-issue
4. meteor npm i --save @firebase/app @firebase/firestore
5. mkdir client
6. echo $'import firebase from "@firebase/app";\nimport "@firebase/firestore";' > ./client/main.js
7. meteor

@benjamn I can look into solving this if you give me a clue of where to start

EDIT: Seems like maybe as simple as updating this line? https://github.com/benjamn/install/blob/master/install.js#L500

[benjamn]

As with many module resolution features, implementing this functionality requires cooperation between static bundling logic (the ImportScanner and Resolver) and the runtime module system (the npm install package). It's not enough to change how module resolution happens at runtime, because the replacement modules have to be included in the bundle, so the bundler has to obey the same logic.

This functionality would be much simpler to implement if the browser object could only override local modules using relative paths, because it's very easy to establish an alias from one module identifier to another module in the bundle, regardless of where the modules are imported. I call this the "relative style" of browser overrides. For example, this aliasing technique is enough to make the tus-js-client work perfectly, since all of its browser overrides are relative. And because the original module is essentially replaced by the alternate module, there is no runtime overhead to this technique.

Unfortunately, the browser object can also override the resolution of packages, but only when those packages are imported by modules in the same package that contains the package.json with the browser field. I call this the "package style" of browser overrides. This is a totally different situation, because we're not simply replacing the package in question with some other module. The overridden package could still be imported by modules outside the package containing the package.json file, and the real package should be returned in those cases. Since the behavior of package resolution depends on the parent module where the require or import appears, every runtime package resolution has to begin with checking to see if there's a package.json file in the current package that has an object-valued browser field that happens to override the given package that is about to be imported. Needless to say, this adds runtime overhead to every single package import, which is especially unfortunate since this feature is so rarely used.

How does this feature work in Webpack and Browserify? Those bundlers statically rewrite all imported module identifiers as numbers, which means require(id) with a dynamic id string is not allowed, because id can't be statically analyzed and rewritten as a number. That's a convenient simplification for a number of reasons, but it means neither of these bundlers fully implements the so-called "spec" for the browser field, because the browser override doesn't apply to dynamic module identifiers, because dynamic module identifiers are not supported in the first place. It's easy to avoid implementing something slowly if you don't support the slow use case at all!

Meteor's module system guarantees any require(id) import will work at runtime, provided the required module is in the bundle. Since Meteor doesn't rewrite your module identifiers as numbers, we don't have the luxury of hacking package overrides into the static bundling process. The override logic has to be available at runtime, so dynamic module identifiers can be handled properly.

Long story short, I think the relative style of browser overrides is totally worth supporting, but the package style simply isn't worth the additional runtime cost and implementation complexity. I realize it may feel weird to implement this functionality halfway, but the relative style appears to be much more common, and it looks like the relative style would be enough to fix most of the examples that have been cited in this issue.

How do folks feel about that? Is it worth supporting the relative style if we don't support the package style? Is that better than the status quo, at least?

[ljharb]

Supporting 80% of the browser field is better than 50%, surely _{(percentages selected are arbitrary, obviously)}

[damonmaria]

As long as it covers more bases than it does now it has to be an improvement. I've struck this issue in Meteor a couple of times (AWS and Algolia packages) and both only use the "relative style".

[aldeed]

@benjamn Thanks as usual for helping us understand the complexities. I agree that supporting the relative style is the primary request here. It seems to be far more common. Would the complexities preclude supporting the false option for ignoring a module, too? That seems like it would be more common than module overrides. But either way, 👍 for supporting relative style.

If it helps, this is as far as I got looking at browserify implementation before I got pulled into other tasks. :) https://github.com/defunctzombie/node-browser-resolve/blob/master/index.js#L153-L176

[benjamn]

The false style is easy as long as the key is relative!

[hwillson]

PR #9311 addresses this issue (by supporting the "relative style" of browser replacements), and has been merged. This should be coming in 1.6.1. Thanks all!

[luchux]

@benjamn I am also having still the error that KayleePop mentioned. Any insights? Seems the object field is still not supported? Any thoughts?

[benjamn]

@luchux My insights are richly detailed in the comments above.

[houshuang]

The latest MobX version uses an object-form of browser in package.json, which Meteor cannot deal with: mobxjs/mobx#1844