search

hapijs / good

hapi process monitoring
Other
525 stars 160 forks source link

Support functions as modules #517

Closed Marsup closed 7 years ago

Marsup commented 8 years ago

Hey,

In our project we stumbled upon a case where the simple string require wasn't enough because npm doing its deduplication work was bringing a different version of the module than we expected.

I don't want to break anyone's code so I kept the old way, which is imho unsafe, we can always remove it later.

I'm not sure the tests are enough but I'll wait for your guidance there.

arb commented 7 years ago

I'm fine with this change and it used to work like this but I took it out during a major bump... I'm curious what two competing versions of modules you have that this problem even came up? Are you using something that wraps something else?

Can you add additional documentation about this change as well?

Marsup commented 7 years ago

We use several composable hapi modules, each coming with its own good, so sometimes version bumps are not applied everywhere, and it causes such conflict. Docs updated.

jaulz commented 7 years ago

Great! When will this be on npm? Btw I need it because I have a webpack build that needs static linking.

FGRibreau commented 7 years ago

:+1: same as @jaulz, I can't pack my app with browserify, because of good current module as string implementation

jaulz commented 7 years ago

@FGRibreau this is already on npm and works 👍

FGRibreau commented 7 years ago

@jaulz I don't see how that could be possible, last version is '7.0.2 and was released 2 months ago '2016-08-27T02:29:16.731Z', while this PR was merged 18 days ago

arb commented 7 years ago

Waiting on @Marsup to review #469 and for changes to #526. You can look at issues and PRs with the 7.1.0 milestone to know what is left before a new version.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.