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

allow non-singleton usage #92

Open
boneskull opened this issue Oct 22, 2020 · 2 comments
Open

allow non-singleton usage #92

boneskull opened this issue Oct 22, 2020 · 2 comments

Comments

@boneskull
Copy link

the register() method keeps whatever was registered in memory, but if two libs were using filing-cabinet at the same time, they may have differing requirements for custom resolvers. maybe a resolver for .foo is different from one to the other, or perhaps one of them should not, under any circumstances, resolve a .foo file.

a way to address this would be to provide a factory function, e.g.:

const filingCabinet = require('filing-cabinet');
const cabinet = filingCabinet.create();
cabinet.register('.foo', myFooResolver);
cabinet({partial: 'bar', filename: 'quux.js', directory: '/some/path'}) // works with .foo files
filingCabinet({partial: 'bar', filename: 'quux.js', directory: '/some/path'}) // does not work with .foo files

I can send a PR to that effect, or if you have a different idea...

@boneskull
Copy link
Author

another way may be to accept an option of an extra resolver mapping, e.g.,

cabinet({partial: 'bar', filename: 'quux.js', directory: '/some/path', resolvers: {
  '.foo': myFooResolver
}});

dunno if that really jives with how people are using this module otherwise

@mrjoelkemp
Copy link
Collaborator

Thanks for the issue! Man, I'm back and forth on this.

So create would return an instance of some inner constructor with its own default lookups?

I think the simpler approach involves changing the module to export a constructor and force you to create instances, but that's a breaking change.

Supporting the dual mode of the default export being just a function call with the tradeoffs you outlined, with the ability to build a new instance for local scoping of state, isn't the end of the world.

If you want to put up a draft for us to review, I'd be happy to take a look and see what the implementation would become.

If you think the breaking interface change of exporting a constructor would be better off, I'm open to that too (should have likely done it that way from the start).

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

3 participants