wowserhq / blizzardry

JavaScript library for mastering the wizardry that is Blizzard's game files
MIT License
55 stars 15 forks source link

Resolve iconv-lite warning #101

Open fallenoak opened 8 years ago

fallenoak commented 8 years ago

Blizzardry uses restructure to parse binary files into something usable in JS. It appears that restructure, in EncodeStream.js, attempts to require a module named iconv-lite:

  try {
    iconv = require('iconv-lite');
  } catch (_error) {}

The require is wrapped in a try catch block, which prevents it from breaking in the event iconv-lite isn't installed. However, Node still emits a warning message to the console:

Module not found: Error: Cannot resolve module 'iconv-lite'

I'd propose we make iconv-lite a dependency of Blizzardry. This will silence the console spam.

Other Notes

While I don't think we're using EncodeStream for anything, iconv-lite may pose a challenge for in-browser usage. It has a wrapper shim that's compatible with Browserify, but I don't expect it'll work out of the box with webpack.

timkurvers commented 8 years ago

Good catch! 👍

It's probably webpack walking the dependency tree and attempting to require iconv-lite. It might be possible to have webpack ignore it, if we don't really need it.

timkurvers commented 8 years ago

The try/catch way of doing things is also mentioned in this webpack issue.

fallenoak commented 8 years ago

That issue talks a bit about optional externals. I gave this a shot in Wowser's webpack.config.js, but no dice:

  externals: {
    'iconv-lite': true
  }

Perhaps because this happens indirectly, through a dependency of Blizzardry, it can't be resolved using optional externals?