IdentityModel / oidc-token-manager

Apache License 2.0
51 stars 36 forks source link

vertx dependency issue #50

Closed thj-dk closed 8 years ago

thj-dk commented 8 years ago

After upgrading to 1.2.0 we're receiving this "vertx" dependency error on build:

WARNING in ./~/oidc-token-manager/dist/oidc-token-manager.js
Module not found: Error: Cannot resolve module 'vertx' in ...\node_modules\oidc-token-manager\dist
 @ ./~/oidc-token-manager/dist/oidc-token-manager.js 6797:20-30

vertx is not referenced as a dependency in package.json. Any ideas?

brockallen commented 8 years ago

Are you using require in your app?

thj-dk commented 8 years ago

Yes, or the new import syntax in ES6.

brockallen commented 8 years ago

Ok, so one of the libraries we're using internally dynamically checks for require and then tries to resolve something called "vertx", which I'm not sure what that is. Here's the line of code:

https://github.com/IdentityModel/oidc-client/blob/master/dist/oidc-client.js#L6813

I guess we could remove this check and fall thru to the next line.

thj-dk commented 8 years ago

Sounds good. I don't get the usage of vertx either.

rpearson-catdevnull commented 8 years ago

I'm also seeing the same warning with webpack on version 1.2.0 - again using require/import like @thj-dk.

rpearson-catdevnull commented 8 years ago

Thought it was worth mentioning that I've managed to get webpack to resolve this dependency. First include vertx3-full in your package.json then in the resolve section of your webpack.config.js add:

resolve: { alias: { vertx: "vertx3-full" } }

Webpack will then be able to resolve the dependency and the warning will go away.

brockallen commented 8 years ago

I'm hesitant to remove it -- that library is a whole lump and I wasn't involved with building it. I plan to do some rework on this library and as part of the process we might be able to modernize some of these dependencies. We'll see...

brockallen commented 8 years ago

Update: I'm working on update oidc-client to the latest of the JWT library. The newest one seems to no longer have this issue.

thj-dk commented 8 years ago

Sounds good @brockallen. I'm looking forward to it!

brockallen commented 8 years ago

It seems that this dependency is no longer in use by the updated version of the JWT library being used in the recent rework. In short, this will be a solved issue once we release (hopefully in the next 1-2 weeks).

thj-dk commented 8 years ago

Great! Thank you very much @brockallen.