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

fix #47 browser hint for 'Long' module #59

Closed
wants to merge 1 commit into from

Conversation

watsoncj
Copy link

Browser hint for webpack and browserify for where to load the "Long" module.

The "index.js": "bytebuffer/dist/ByteBufferAB.js" line is effectively doing the same thing as the previous "browser": "dist/ByteBufferAB.js". It's used in conjunction with the main option and is necessary since the browser property is overloaded.

The Long mapping should help avoid renaming the AMD module.

@dcodeIO
Copy link
Member

dcodeIO commented Sep 25, 2015

I see, but index.js contains some code that is not desirable to have in the browser. index.js targets node, that's why there is "browser": "dist/ByteBufferAB.js". Any ideas? Or isn't that a problem at all because every tooling around (not just webpack) will resolve index.js that way?

@watsoncj
Copy link
Author

This mapping causes ByteBufferAB.js to be bundled instead of index.js for browser

Testing this change in webpack; ByteBufferAB.JS was bundled. index.js was NOT bundled.

The comments in the "browser field spec" indicate that browserify implements the same behavior.

I'm still unclear if relative paths are resolved identically by all bundlers (webpack & browserify).

@watsoncj
Copy link
Author

The browserify handbook documents a slightly different behavior with relative paths. This PR needs to be tested in browserify before merging.

"browser": "dist/ByteBufferAB.js",
"browser": {
"index.js": "bytebuffer/dist/ByteBufferAB.js",
"Long": "bytebuffer/node_modules/long/dist/Long.js"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look browserify safe, also the Long value should not be in this module. It should be resolved by long's package.json.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @englercj, the issue I was trying to address is getting webpack to use node_modules/long as the provider of Long.

@watsoncj
Copy link
Author

watsoncj commented Oct 7, 2015

This will break browserify and isn't a great solution. See comments about resolve aliases and script-loaders on #47.

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

Successfully merging this pull request may close these issues.

None yet

3 participants