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

Resolve iconv-lite warning #101

Open
fallenoak opened this issue Jun 14, 2016 · 3 comments
Open

Resolve iconv-lite warning #101

fallenoak opened this issue Jun 14, 2016 · 3 comments

Comments

@fallenoak
Copy link
Member

fallenoak commented Jun 14, 2016

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
Copy link
Member

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
Copy link
Member

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

@fallenoak
Copy link
Member Author

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?

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