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

Javascript require of npm module ending with .JS does not work #49

Closed
dazraf opened this issue Apr 28, 2016 · 3 comments
Closed

Javascript require of npm module ending with .JS does not work #49

dazraf opened this issue Apr 28, 2016 · 3 comments
Labels

Comments

@dazraf
Copy link

dazraf commented Apr 28, 2016

Hello

This may be a bug with vertx-lang-js.

Reproducer

In a new directory:

npm init
npm install bignumber.js 
npm install web3

in a test javascript file, try:

require('web3') // or 
require('bignumber.js') // or
var BigNumber = require('node_modules/bignumber.js/bignumber.js');

Summary

It appears that relatively common npm modules have a .js suffix in their module name (!)

e.g. bignumber.js

I wouldn't normally want to use the above module, except that other modules, that I do want to use, require it

e.g. ethereum web3

Hence, given ...

var Web3 = require('web3')

...vertx-lang-js throws with:
java.lang.RuntimeException: java.io.FileNotFoundException: /Users/Fuzz/tmp/vertx-js-demo/node_modules/web3/node_modules/bignumber.js (Is a directory)

Detail

What's interesting is that web3 itself has embedded its dependencies as a full copy within its distribution.
This seems to almost work with vertx-lang-js, failing only because it interprets a path ending with .js as being a javascript file (quite reasonably, but not the way node.js is resolving it, it seems).

So, looking at vertx-lang-js, I found the following in jvm-npm.js

  Require.resolve = function (id, modParent) {
    var roots = findRoots(modParent);
    for (var i = 0; i < roots.length; ++i) {
      var root = roots[i];
      var result = resolveClasspathModule(id, root) ||
        resolveAsFile(id, root, '.js') ||
        resolveAsFile(id, root, '.json') ||
        resolveAsDirectory(id, root) ||
        resolveAsNodeModule(id, root);
      if (result) {
        return result;
      }
    }
    return false;
  };

In contrast, node appears to prefer module and directory resolution before file resolution.

regards
Fuzz.

@pmlopes pmlopes self-assigned this May 3, 2016
@pmlopes
Copy link
Member

pmlopes commented May 3, 2016

This is more tricky than just re-order the loaders, since most usages of vertx will be a fatjar and therefore modules will be loaded from the classpath. the issue here is that the classloader will return a valid reference to a directory and since it is just a input stream it will not be easy to identify if this should be a module or not...

@cescoffier
Copy link
Member

Until java 9 (which change this), the classloader is a urlclassloader, so url with a trailing '/' are directories.

@pmlopes pmlopes removed their assignment May 3, 2016
@pmlopes
Copy link
Member

pmlopes commented Dec 9, 2018

this has been fixed on https://github.com/reactiverse/es4x back porting is not possible just because it would introduce behavior changes to the loader. The current loader is not fully compatible with commonjs, while the fixed one is and that's why it can make the distinction between files and directories.

@pmlopes pmlopes added the wontfix label Dec 9, 2018
@pmlopes pmlopes closed this as completed Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants