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

Cannot be used inside non-main module #5

Closed
Raynos opened this issue Feb 18, 2015 · 9 comments · Fixed by #11
Closed

Cannot be used inside non-main module #5

Raynos opened this issue Feb 18, 2015 · 9 comments · Fixed by #11

Comments

@Raynos
Copy link
Contributor

Raynos commented Feb 18, 2015

Let's say I have a package called foo and it has three modules, bar, baz, bob.

When I use codependency inside foo/bar it will not work because of https://github.com/Wizcorp/codependency/blob/master/index.js#L183-L188

codependency assumes it's only imported in the main module of the package.

I added a hack to my local module

var packageModule = safeRequire(dirname(pkgPath))
var filePackageModule = safeRequire(
    pathJoin(dirname(pkgPath), pathBasename(baseModule.filename))
);

if (
    packageModule !== baseModule.exports &&
    filePackageModule !== baseModule.exports
) {
    throw new Error(
        'No package.json found that resolves to "' + baseModule.filename + '" ' +
        '(found instead: "' + dirname(pkgPath) + '").'
    );
}

To allow it to detect either the main package is equal to me or the dirname + filename is equal to me.

However this does not support packages that have modules called lib/foo or test/mocks/bar. I do not know what heuristic we can use to support those.

@ronkorving
Copy link
Collaborator

I'm sorry. I'm not sure if I completely follow. Could you give a more complete code snippet of how you're using codependency in this scenario? (or how you would like to use it)

@Raynos
Copy link
Contributor Author

Raynos commented Feb 19, 2015

I have a package called xutil that has two files in it. xutil/extend and xutil/copy

If the application developer does require('xutil') it will throw an exception because there is no xutil/index.js.

xutil codepends on bob in both files, the codependency code basically assumes that there is an index.js as part of it's verification & checking process.

@ronkorving
Copy link
Collaborator

Ah, so normally, people will always require xutil/extend or xutil/copy, but not xutil, which is not requireable? I guess you're not talking about https://www.npmjs.com/package/xutil btw, since that code doesn't match what you describe?

@Raynos
Copy link
Contributor Author

Raynos commented Feb 23, 2015

@ronkorving You are correct. I made it an arbitrary example.

@ronkorving
Copy link
Collaborator

ronkorving commented Aug 16, 2017

@Raynos Sorry, I'm cleaning up some open issues I haven't looked at in a while. Apologies. You mention that codependency looks for an index.js, but (today at least) it doesn't. Is this still an issue?

Also, if you could have a look at #11 and give your feedback on that?

@spencerhakim
Copy link

@ronkorving I'm hitting this issue, too. Regarding index.js: codependency assumes a single root module per package (which obviously isn't a safe assumption). This is usually set by the main field in package.json, but falls back to index.[js|json|node] if none is specified, which is part of the overall functionality of require(). I think the safer thing to do here is to just find the closest package.json for any module passed in and then work from there, or even let users pass in the path to their package.json instead.

@bertho-zero
Copy link
Contributor

I think this pull request will fix this issue: #14

@Raynos @spencerhakim if you have the opportunity to try.

@alasdairhurst
Copy link

alasdairhurst commented Nov 15, 2017

I have a similar issue where i'm using this for an eslint config. The module includes multiple configs which are in my project eslint-config-myproject as
/react.js
/mocha.js

i would then use eslint-config-myproject as an eslint config for myapp.

you would only get warned of missing peer dependencies if you use one or more of these configs, but the configs are optional. (they have respective plugin-react and plugin-mocha peer dependencies )

since myapp doesn't use react, but does use mocha, I specify "myproject/mocha" in the plugins for eslint to load.

It's eslint itself that loads this module and it's configs and plugins.

when i try to do

var codependency = require('codependency');
var requirePeer = codependency.register(module);

requirePeer('eslint-plugin-mocha');

in /mocha.js

i get the following error when running eslint in the parent myapp application

Error: No package.json found that resolves to "C:\myapp\node_modules\eslint-config-myproject\mocha.js" (found instead: "C:\myapp\node_modules\eslint-config-myproject").

Surely the package.json that it found in C:\myapp\node_modules\eslint-config-myproject is correct and is the one it should use, and shouldn't be an error?

@ronkorving
Copy link
Collaborator

ronkorving commented Nov 15, 2017

@alasdairhurst I think a simple solution to that would be to add an option to register's options called strict (this currently does not exist). If that is set to false (I think it should default to true), we could then pass that here. Passing false there instead of true will probably make your use-case work.

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

Successfully merging a pull request may close this issue.

5 participants