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

Support functions as modules #517

Merged
merged 2 commits into from Oct 13, 2016
Merged

Support functions as modules #517

merged 2 commits into from Oct 13, 2016

Conversation

@Marsup
Copy link
Member

Marsup commented Sep 21, 2016

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

This comment has been minimized.

Copy link
Contributor

arb commented Sep 26, 2016

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

This comment has been minimized.

Copy link
Member Author

Marsup commented Sep 26, 2016

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.

@arb arb added this to the 7.1.0 milestone Sep 26, 2016
@arb arb added the feature label Sep 26, 2016
@arb arb self-assigned this Sep 26, 2016
@jaulz

This comment has been minimized.

Copy link

jaulz commented Oct 13, 2016

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

@arb arb merged commit ae1778b into master Oct 13, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@arb arb deleted the function-module branch Oct 13, 2016
@FGRibreau

This comment has been minimized.

Copy link

FGRibreau commented Oct 30, 2016

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

@jaulz

This comment has been minimized.

Copy link

jaulz commented Oct 31, 2016

@FGRibreau this is already on npm and works 👍

@FGRibreau

This comment has been minimized.

Copy link

FGRibreau commented Oct 31, 2016

@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

This comment has been minimized.

Copy link
Contributor

arb commented Oct 31, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.