keystonejs / keystone-classic

Node.js CMS and web app framework
http://v4.keystonejs.com
MIT License
14.65k stars 2.21k forks source link

Add ability to import a single file to Keystone #4931

Closed autoboxer closed 5 years ago

autoboxer commented 5 years ago

Description of changes

This is something I created for my own repo that allows loading of Keystone models from different component directories instead of requiring them to exist in the same models directory.

See lines 79-83 of https://github.com/mareinc/mare/blob/update-application-structure/keystone.js for an example.

Related issues (if any)

No related issues

Testing

Tests are currently failing, however, this exposes new functionality without changing how existing Keystone functionality operates.

laurenskling commented 5 years ago

I think this is a quite specific wish/feature. It doesn't really do anything Keystone-y, it's just a helper function, right? I'm unsure it is needed within the core of Keystone.

autoboxer commented 5 years ago

@laurenskling, that is correct, it doesn't do anything related to Keystone functionality, however, it allows users to load files into a Keystone project in a different way. Keystone core has a function called importer which is used to store paths to models on the Keystone object. This function does the same thing, but doesn't require all files to be in the same directory. I'm using it because I don't like the monolith structure with all my models in a single directory. Instead, I prefer a directory for each component, and each component directory to contain it's own model(s). With the current functions Keystone is using to load these files, it's not possible. This helper function makes it possible, and because It's adding an additional option to what exists in Keystone core, I wasn't sure of a better place to put it.

JedWatson commented 5 years ago

@autoboxer I'm inclined to push back on this in like with what @laurenskling said - this seems like something that would be better dealt with in "user land" than keystone core.

Unless I'm missing something (?) it's just a helper that requires a file relative to the cwd of your keystone app - is there any specific reason that in your linked example, you shouldn't just replace the function call with a normal require? it's not using the returned modules at all, or keeping them on the keystone instance, so this feels very much like something that could live elsewhere.

JedWatson commented 5 years ago

As discussed in our maintainers chat, we're going to decline this one