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

Module warnings unavoidable when importing inside config dependency #1298

Open
DesignByOnyx opened this Issue Nov 8, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@DesignByOnyx
Copy link
Contributor

DesignByOnyx commented Nov 8, 2017

I have a file I use in both a config dependency and within my app:

// config dependency:
const config = require('../shared/config');

// throughout client code
import config from '~/shared/config';

This results in the "module instantiated twice" warning in a way which is unavoidable unless we abandon the use of tilde/project-name in all of our project code.

@matthewp

This comment has been minimized.

Copy link
Member

matthewp commented Nov 9, 2017

This is a dupe of #1285. Will be fixed in the next patch release most likely.

@matthewp matthewp closed this Nov 9, 2017

@justinbmeyer

This comment has been minimized.

Copy link
Contributor

justinbmeyer commented Nov 9, 2017

@justinbmeyer

This comment has been minimized.

Copy link
Contributor

justinbmeyer commented Nov 9, 2017

To be more specific, we should be warning about this case I think.

  1. He has a file that he loads as a config dep and a normal dep. @matthew think it's possible to have a no-npm-resolution:path/to/module type thing?

@matthewp matthewp reopened this Nov 9, 2017

@matthewp

This comment has been minimized.

Copy link
Member

matthewp commented Nov 9, 2017

We can discuss but this still sounds like the same issue. I'm not sure what feature you are requesting. But I don't think people should have to be aware of some new syntax to avoid false-positive warning messages. Better just to change the log-level and turn on more verbose logging if you want to see these messages.

@justinbmeyer

This comment has been minimized.

Copy link
Contributor

justinbmeyer commented Nov 9, 2017

But I don't think people should have to be aware of some new syntax to avoid false-positive warning messages

This isn't false positive. With config dependencies, if you import node_modules/foo/bar, the module name is node_modules/foo/bar.

After npm plugin has loaded, you should import foo/bar.

Another way to fix this would be to support post-npm config dependencies.

@matthewp

This comment has been minimized.

Copy link
Member

matthewp commented Nov 13, 2017

This warning is probably not visible now.

@frank-dspeed

This comment has been minimized.

Copy link
Contributor

frank-dspeed commented Jan 20, 2018

@matthewp what do you think about justins suggestion i mean making this even is essential as this is in other coding situations a standard case to have different paths that result in the same static path if we work with more big code bases and plug ins to modules coded by different coders.

@frank-dspeed

This comment has been minimized.

Copy link
Contributor

frank-dspeed commented Jan 20, 2018

@justinbmeyer @matthewp i am also suggesting implementing and using this hooks
https://nodejs.org/api/esm.html#esm_resolve_hook
this is the new nativ node way to handle multiple module formarts and path related stuff as also dynamic loading

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