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

Issue when bundling with webpack #172

Closed
Apidcloud opened this issue Sep 20, 2017 · 12 comments
Closed

Issue when bundling with webpack #172

Apidcloud opened this issue Sep 20, 2017 · 12 comments

Comments

@Apidcloud
Copy link

Good evening!

While attempting to use your package with webpack, I've noticed an issue in index.js:

ERROR in ./node_modules/mime/index.js
Module not found: Error: Can't resolve './types/other' in 'test-project\node_modules\mime'
 @ ./node_modules/mime/index.js 4:55-79

ERROR in ./node_modules/mime/index.js
Module not found: Error: Can't resolve './types/standard' in 'test-project\node_modules\mime'
 @ ./node_modules/mime/index.js 4:26-53

Doing specific requires does the trick (note the extension):

module.exports = new Mime(
  require('./types/standard.json'),
  require('./types/other.json')
);

Let me know if you want me to do a PR.

Thanks!

@broofa
Copy link
Owner

broofa commented Sep 20, 2017

In looking at this, I realized that my comment on #124 that referred to index.js was misleading. I've provided more correct code in this comment

Please give that a try and see if it fixes your issue. If not, let me know and I'll reopen this.

[As an aside, webpack is able to load json files for me without the need for the .json extension.]

@broofa broofa closed this as completed Sep 20, 2017
@Apidcloud
Copy link
Author

Apidcloud commented Sep 20, 2017

I was only able to fix it by adding the extensions manually inside node_modules.

Some details regarding my env:

  • Windows 10
  • node v8.5.0
  • npm v5.3.0
  • yarn v1.0.2
  • webpack v3.6.0

@broofa
Copy link
Owner

broofa commented Sep 20, 2017

Can provide detailed steps for repro'ing this?

@Apidcloud
Copy link
Author

Apidcloud commented Sep 20, 2017

Adding '.json' to the configuration in webpack also solves the issue.

resolve: {
    // Look for modules in .js files first
    extensions: [".js", ".json"],

Still, it's a workaround and I would say it could be avoided if mime package used the extension in the require statements in the first place. It doesn't feel right to adjust a bundler configuration based on a 3rd party package's dependencies (i.e., it's not even something to do with mime directly, but a dependency within. I don't use json files in my project, so adding that to webpack purely because of that doesn't feel right at all).

I guess a repository with a standard webpack configuration without the extension '.json' should reproduce the errors mentioned above.

@broofa
Copy link
Owner

broofa commented Sep 26, 2017

I'm afraid I have to conclude this is an issue with your project/webpack configuration. When I try a basic "hello world" test of webpack's JSON loading behavior, it works fine:

kieffer@MacBook-Pro-3$ pwd
/Users/kieffer/work

kieffer@MacBook-Pro-3$ npm install webpack

...

+ webpack@3.6.0
added 365 packages in 35.241s

kieffer@MacBook-Pro-3$ echo "{\"hello\": \"world\"}" > foo.json

kieffer@MacBook-Pro-3$ echo "require('./foo')" > entry.js

kieffer@MacBook-Pro-3$ webpack ./entry.js bundle.js
Hash: 4c7fbae89938019cd2d7
Version: webpack 3.6.0
Time: 76ms
    Asset     Size  Chunks             Chunk Names
bundle.js  2.61 kB       0  [emitted]  main
   [0] ./entry.js 17 bytes {0} [built]
   [1] ./foo.json 34 bytes {0} [built]

kieffer@MacBook-Pro-3$ cat bundle.js

[*snip*]

__webpack_require__(1)


/***/ }),
/* 1 */
/***/ (function(module, exports) {

module.exports = {"hello":"world"}

/***/ })
/******/ ]);

If I try the same thing when requiring the mime module, it also works:

kieffer@MacBook-Pro-3$ npm install mime
npm WARN saveError ENOENT: no such file or directory, open '/Users/kieffer/work/package.json'
npm WARN enoent ENOENT: no such file or directory, open '/Users/kieffer/work/package.json'
npm WARN work No description
npm WARN work No repository field.
npm WARN work No README data
npm WARN work No license field.

+ mime@2.0.3
added 1 package in 3.658s

kieffer@MacBook-Pro-3$ echo "require('mime')" > entry.js

kieffer@MacBook-Pro-3$ webpack ./entry.js bundle.js
Hash: c40eefc044fb6dcfdb83
Version: webpack 3.6.0
Time: 117ms
    Asset     Size  Chunks             Chunk Names
bundle.js  36.3 kB       0  [emitted]  main
   [0] ./entry.js 16 bytes {0} [built]
    + 4 hidden modules

kieffer@MacBook-Pro-3$ cat bundle.js

[*snip*]

/***/ }),
/* 3 */
/***/ (function(module, exports) {

module.exports = { [*contents of types/standard.json snipped*] }

/***/ }),
/* 4 */
/***/ (function(module, exports) {

module.exports = { [*contents of types/other.json snipped*] }

/***/ })
/******/ ]);

Note, too, that JSON-loading behavior of require() is well documented. I'll specifically refer you to the bit about "If X.json is a file, parse X.json to a JavaScript Object. STOP". mime is hardly the only project relying on this. (i.e. fixing it here is likely just a band-aid that whatever the underlying packager configuration issue is, and you'll run into this again with some other project.)

@broofa
Copy link
Owner

broofa commented Sep 26, 2017

So, I'm going to go out on a limb here and offer some unsolicited advice:

  1. For issues like this, that involve projects used by literally 100Ks of users and projects, if you have an issue that looks like it would break things for all those other folks, but nobody else is reporting it... well... odds are it's something you're doing wrong. I've learned the hard way that assuming someone else is to blame is a good way of putting your foot in your mouth.
  2. Speaking of which, a good way to avoid such embarrassment is to get in the habit of including reproducible test cases as part of your initial bug report. Oftentimes, the process of creating such a test case will identify what the problem is, and you'll gain a deeper understanding of how the code you depend on works along the way. It's a win-win regardless of what you find.

I know that may be a bit of hubris on my part. If I'm wrong, and this really is an issue in mime, I'll humbly apologize. But I'm not seeing the issue in question, and I've done everything I can to try to reproduce it. Without a reproducible test case, that's actually above and beyond what most project maintainers would do. At this point it's on you to figure out what's wrong and why.

@Apidcloud
Copy link
Author

Apidcloud commented Sep 26, 2017

Thanks for taking the time to dig on this and for the advice. I apologize for not referencing a test case right away. I should have done so in the first place.

Just recreated the issue in a test project, so you can now take a look at it: https://github.com/Apidcloud/webpack-babel/tree/issueNodeMime

The use of mime is done in src/core/snake, and you can reproduce such errors by running $ npm run build

As aforementioned, it can be solved by specifically resolving .json files with webpack, but doing such thing due to 3rd party package's internal dependencies doesn't feel quite right.

Thanks again!

@broofa
Copy link
Owner

broofa commented Sep 26, 2017

npm run build works fine for me. But... it's not trying to pull in mime anywhere. At least, not that I can see. 😕

kieffer@MacBook-Pro-3$ pwd
/Users/kieffer/repos/webpack-babel

kieffer@MacBook-Pro-3$ git log --oneline
c730cfb (HEAD -> master, origin/master, origin/HEAD) Upgrade to webpack 3
e6250e3 Change bundle target to UMD
cba20cf Update package.json
436aab5 Remove unnecessary code for boilerplate
09959cc Fix live-server scripts
1962a39 Fix webpack configuration by excluding node_modules from module rules
e9fb370 Add http-server script
e502ac8 Create README.md
1c18b36 Add webpack config and sample es6 case use
f93506b Add initial files

kieffer@MacBook-Pro-3$ npm --version
5.4.2

kieffer@MacBook-Pro-3$ node --version
v7.10.0

kieffer@MacBook-Pro-3$ webpack --version
3.5.2

kieffer@MacBook-Pro-3$ npm run build

> webpack-babel@1.0.0 build /Users/kieffer/repos/webpack-babel
> webpack -d --config config/webpack.config.js

Hash: 6a266f9710384dd1f0be
Version: webpack 3.5.2
Time: 1031ms
                   Asset    Size  Chunks                    Chunk Names
webpack-babel.browser.js  329 kB       0  [emitted]  [big]  main
  [51] multi index.js 28 bytes {0} [built]
  [52] ./src/index.js + 3 modules 3.39 kB {0} [built]
    + 98 hidden modules

kieffer@MacBook-Pro-3$ grep --exclude-dir=node_modules --exclude=package-lock.json -r mime .

kieffer@MacBook-Pro-3$

@broofa
Copy link
Owner

broofa commented Sep 26, 2017

Tried with variants of NODE_ENV=es6 and NODE_ENV=production. Same results.

@Apidcloud
Copy link
Author

I believe you are in master branch. Try to checkout to issueNodeMime

@broofa
Copy link
Owner

broofa commented Sep 26, 2017

Checked out issueNodeMime branch. Had some weirdness with npm not installing devDependencies (see below). Eventually got it repro'ing your error though.

Looks like the problem is that you're supplying a custom definition of config.resolve.extensions. The default value of that field includes json as an extension, but in your definition, you only specify js and jsx.

Webpack is almost certainly "working as designed" here. Regardless of whether this is a good thing, this is well-documented behavior for CommonJS module loading. Omitting json as an extension almost certainly breaks many other modules beyond mime. So fixing the problem here (and breaking with the "no-extension" convention) isn't something I'm particularly interested in doing.


kieffer@MacBook-Pro-3$ npm install
added 4 packages in 2.609s

kieffer@MacBook-Pro-3$ ls node_modules/
babel-runtime mime

kieffer@MacBook-Pro-3$ npm run build

> webpack-babel@1.0.0 build /Users/kieffer/repos/webpack-babel
> webpack -d --config config/webpack.config.js

sh: webpack: command not found
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn
npm ERR! webpack-babel@1.0.0 build: `webpack -d --config config/webpack.config.js`
npm ERR! spawn ENOENT
npm ERR!
npm ERR! Failed at the webpack-babel@1.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kieffer/.npm/_logs/2017-09-26T16_41_08_608Z-debug.log

kieffer@MacBook-Pro-3$ npm install --only=dev

> fsevents@1.1.2 install /Users/kieffer/repos/webpack-babel/node_modules/fsevents
> node install

[fsevents] Success: "/Users/kieffer/repos/webpack-babel/node_modules/fsevents/lib/binding/Release/node-v51-darwin-x64/fse.node" is installed via remote

> uglifyjs-webpack-plugin@0.4.6 postinstall /Users/kieffer/repos/webpack-babel/node_modules/uglifyjs-webpack-plugin
> node lib/post_install.js

added 668 packages in 9.939s

kieffer@MacBook-Pro-3$ npm run build

> webpack-babel@1.0.0 build /Users/kieffer/repos/webpack-babel
> webpack -d --config config/webpack.config.js

Hash: d3f76fa6a16af1f4350d
Version: webpack 3.5.2
Time: 1241ms
                       Asset    Size  Chunks                    Chunk Names
webpack-babel.browser.min.js  337 kB       0  [emitted]  [big]  main
  [51] multi index.js 28 bytes {0} [built]
  [52] ./src/index.js + 3 modules 3.51 kB {0} [built]
    + 100 hidden modules

ERROR in ./node_modules/mime/index.js
Module not found: Error: Can't resolve './types/standard' in '/Users/kieffer/repos/webpack-babel/node_modules/mime'
 @ ./node_modules/mime/index.js 4:26-53
 @ ./src/core/snake.js
 @ ./src/core/index.js
 @ ./src/index.js
 @ multi index.js

ERROR in ./node_modules/mime/index.js
Module not found: Error: Can't resolve './types/other' in '/Users/kieffer/repos/webpack-babel/node_modules/mime'
 @ ./node_modules/mime/index.js 4:55-79
 @ ./src/core/snake.js
 @ ./src/core/index.js
 @ ./src/index.js
 @ multi index.js
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! webpack-babel@1.0.0 build: `webpack -d --config config/webpack.config.js`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the webpack-babel@1.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kieffer/.npm/_logs/2017-09-26T16_38_18_589Z-debug.log

@Apidcloud
Copy link
Author

Apidcloud commented Sep 26, 2017

Glad you found the root of the problem. That makes sense despite not being aware of the webpack defaults to .js and .json.

Nevertheless, I guess (part of) the point remains the same: on whether this package (and possibly many others using json files) couldn't mitigate such (breaking) behaviour by adding the extension.

I understand it's not a problem per se, but the package is still depending on a specific external configuration (even if the default), which could be avoided I would argue.

Again, not saying it's this package problem. There are multiple solutions to make it work, so it ends up being just a suggestion.

Thanks again!

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

No branches or pull requests

2 participants