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

Potential use case: mocking internals #46

Open
MylesBorins opened this issue Mar 18, 2020 · 7 comments
Open

Potential use case: mocking internals #46

MylesBorins opened this issue Mar 18, 2020 · 7 comments

Comments

@MylesBorins
Copy link
Member

MylesBorins commented Mar 18, 2020

A thought I had, perhaps module attributes could be used to mock unexposed parts of modules. This is a serious current gap for testing and something we are trying to figure out how to best solve in node core... this would not have to be standardized in ecma262 imho, but would be a good one to explore

import feature from './feature.mjs' with {
  internal: {
    express: './path/to/mocked/express.mjs'
  }
}

edit:

dynamic example for completeness

const feature = import('./feature.mjs', {
  internal: {
    express: './path/to/mocked/express.mjs'
  }
});
@devsnek
Copy link
Member

devsnek commented Mar 18, 2020

that seems inconsistent (glhf if you forget to put the big mock override attribute on one of your imports) and kind of weird (why not just directly import the mocked file, since you're writing it down directly in static source text)

@jridgewell
Copy link
Member

jridgewell commented Mar 18, 2020

why not just directly import the mocked file, since you're writing it down directly in static source text

I don't think that would actually help mocking? If express.mjs exports a foo function that accesses disk, and feature.mjs imports and uses that, there's no way for me to mock that.

@ljharb
Copy link
Member

ljharb commented Mar 18, 2020

Mocking would have to happen at top-of-tree, and module attributes are scattered amidst the tree. I think this would either require a pre-graph loader, or spec-level syntax, totally unrelated to module attributes (iow, mocking is not a per-module concern, it's a per-graph concern, and module attributes are exclusively for per-module concerns)

@mcollina
Copy link

I think we need a little bit more thought into this. I think mocking is not just replacing one file with another, but rather providing a way to inject full behavior when loading something.

If we consider:

const feature = import('./feature.mjs', {
  internal: {
    express: {
      exports: ['foobar', 'default'],
      async function load (args) {
        // this should be what is resolved to a module.

      }
    } 
  }
});

iow, mocking is not a per-module concern, it's a per-graph concern, and module attributes are exclusively for per-module concerns

Can you please expand on this?
My main concern in making it a per-graph means that things could not be easily garbage-collected, which would be needed for the above to work.

@ljharb
Copy link
Member

ljharb commented Mar 19, 2020

@mcollina what i mean is, when you're mocking a specifier, say "lodash" - it's not automatically appropriate to mock all imports of lodash in the entire graph (because your mock might only meet the needs of some of the things depending on this version of lodash), nor is it automatically approrpriate to only mock imports of lodash in a specific subgraph (because you don't want duplicates in your browser bundle, for example, if your mock could be used to replace all instances of lodash in the tree).

Thus, the decision of what to mock, and for which subgraphs, can only be made with knowledge of the entire dependency graph - it's an app-level decision, not a decision that an individual module author can possibly make. Thus, code that goes inline into a module can not possibly meet, in a general sense, the use cases for mocking, because the module author isn't equipped to make that decision (for the cases where they are, they could simply import a different thing, without needing mocks)

@mcollina
Copy link

I don't think it's necessary to mock the whole graph, but rather only what the single level module imports. From my point of view it's not a graph-level decision, but rather just at the top of the tree.

@justinmchase
Copy link

const feature = import('./feature.mjs', {
  modules: {
    'file://full/url/to/module.js': {
      get async resolve (args) {
        // this should be what is resolved to a module.
      }
    } 
  }
});

When you do a dynamic import you want to be able to override any arbitrary module that might get imported as a result of this import. If the module you're trying to overload was previously loaded and cached, you would want it to be re-resolved for this dynamic import, as well as all modules who directly or indirectly would resolve this module.

For testing you need this, you will need that module to be mocked or not mocked by new instances on every import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants