-
Notifications
You must be signed in to change notification settings - Fork 154
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
WebPack deployment error: Cannot resolve module 'Long' #47
Comments
Relevant source file: https://github.com/dcodeIO/ByteBuffer.js/blob/master/src/ByteBufferAB.js Which initializer is used by webpack? |
Any findings? |
Sorry, we have no idea how to answer your question. It would be good to get a proper deployment though, webpack shows a warning because I have built it by hand using browserify. I did see a reference to SlowBuffer, I suspect that may be due to improper deployment. |
I have updated Long.js to export the dist file directly. Not sure if this makes a difference, so I tested it with the change. I got are two warnings about name conflicts on "Long" and "long", but it worked: // testfile.js
var ByteBuffer = require("dist/ByteBufferAB.js");
console.log(ByteBuffer);
Returns both ByteBuffer and ByteBuffer.Long properly populated. |
I also added a webpack.config.js now which points to the correct entry point. Not sure if this becomes evaluated though. I think it's important that, for browser usage, |
I cloned master into
|
When cloning master, make sure to run |
I'm also having this issue. I'm going to dig into it after i fix my deploy and see if i can figure out what's going on. It will only occur on systems with case sensitive filesystems. |
@jcalfee 's test will pass with warnings on OSX (with a case-insensitive file system) The test fails when run on a case-sensitive file system. OSX
|
It's either require("long") with CommonJS (module name) or a plain/AMD "Long.js" dependency (script name). Does webpack install dependencies automatically or do you have to add them manually? In case of the latter, it's recommended to rename the files to lowercase. I assume, ideally, that using lowercase everywhere would be better practice, but that requires a bit of work on docs and stuff and I'd have to do this for all of the libraries involved simultaneously. |
For webpack it's common to install dependencies through npm or possibly bower. Renaming the files might not be feasible in the case where the dependencies are managed. I think the package.json |
Sounds good, but is that file path always known? Like with npm, modules are in |
I may be wrong about people using bower with webpack and browserify. Common case is npm installs bytebuffer and it's node_modules. |
Proposed fix here: #59 |
A resolve alias seems to be a good solution if you just need to use ByteBuffer and Long (but not protobufjs).
Another good option (especially for protobufjs since webpack can't bundle it's usage of require) is the script-loader in combination with externals:
|
Problem here is what webpack handles both AMD-style and CommonJS style definitions
There is two module inclusions, one In order to fix that they should be both declared with same name, and since CommonJS require is filesystem based, I think they both should be lowercase. changing AMD code to `define(["long"], factory);' removes an error, but I am not sure if this is correct for AMD loaders. There are workarounds for that, like specifying CamelCase alias for webpack pointing to node_modules:
but I believe making library webpack-friendly worth it, taking into account rise in webpack's popularity and what it is not so easy to figure out what happens then you get that error first time (it is rare) |
btw, the same problem in |
That is a long standing issue. Actually, the libraries are correctly named Long, ByteBuffer etc., but npm doesn't allow upper case packages, which results in this situation. Ideally, I should rename everything to lowercase, but I haven't got around to this as it would already break so much stuff. There is an example config with resolve/alias on the ProtoBuf repository, but sadly every single project has to include this on its own atm. |
This is much better than having to use the script-loader to include protobufjs or bytebuffer. The script-loader approach can't set globals in IE10. |
Can you have a pointer to both names Long, long, ByteBuffer, bytebuffer? |
Yes, as long as you're ok having multiple instances of the ByteBuffer or Long:
|
Started conversion to lower case with Long. Will become 3.X. |
It is ok if they are just instance pointers .. but I would not want multiple instances... |
Here goes ByteBuffer: 53fcc7c |
And ProtoBuf: protobufjs/protobuf.js@642560c |
Great! Many thanks. Looks like issue can be closed |
Please publish updated libraries to npm. |
Published bytebuffer, currently checking protobuf. |
Well this error keeps occurring to me, and none of the answers above solves it. |
There is an issue deploying ByteBuffer 3.5.4 under webpack. Here are steps to reproduce:
Setup:
Run:
The text was updated successfully, but these errors were encountered: