support 'mixinsources' option #131
Conversation
| mixins = mixins.filter(function(m) { | ||
| return (includeMixins.indexOf(m.name) === -1 ? false : true); | ||
| }); | ||
| } |
There was a problem hiding this comment.
I am proposing to move this filtering code out of "loadMixins" to keep "loadMixins" simpler and focused on doing one thing only.
PradnyaBaviskar
force-pushed
the
lb-issue-79-mixinsources
branch
from
7099aa7
to
4e15c52
Compare
| // Filter in only mixins, that are used in models | ||
| mixinsFromMixinSources = filterMixins(mixinsFromMixinSources, modelMixins); | ||
|
|
||
| mixins = mixins.concat(mixinsFromMixinSources); |
There was a problem hiding this comment.
This may create duplicate items in the mixins array, when the same mixin is loaded from mixinDirs and mixinSources, is that correct? In which case you should de-duplicate the instructions.
PradnyaBaviskar
force-pushed
the
lb-issue-79-mixinsources
branch
from
4e15c52
to
7f11a15
Compare
|
@bajtos - Addressed your comments above and have also added a few more test cases. Can you please review? |
| var files = options.mixins || []; | ||
| var mixins = loadMixins(appRootDir, mixinDirs, files, extensions, options); | ||
| if (mixins === undefined) return; |
There was a problem hiding this comment.
I find this difficult to grasp at the first sight, it looks like loadMixins should be modifying the files variable.
|
The implementation looks good in general now, although I find it difficult to understand the implementation in compiler. Can you please try to simplify it a bit more, using my comments above for inspiration? Don't worry about it too much. If the result does not look better than the current solution or it takes too much time, then I am ok to land the patch as it is now. |
PradnyaBaviskar
force-pushed
the
lb-issue-79-mixinsources
branch
2 times, most recently
from
d460161
to
77c8c60
Compare
| return mixins; | ||
| } | ||
|
|
||
| function findMixinDefinition(appRootDir, sourceDirs, extensions) { |
There was a problem hiding this comment.
nitpick: please rename to findMixinDefinitions (plural).
- By default looks for mixinsources in directory - Loads only mixins used through models
PradnyaBaviskar
force-pushed
the
lb-issue-79-mixinsources
branch
from
77c8c60
to
53f5182
Compare
|
Reworked on |
|
Awesome, landed! |
|
I am facing an issue as "column "columnname" does not exist", when i try to find any data from my Person table.I have 'name' and 'role' fields in my Person table, and 'status' is not present in my table(not require same in table).But my model.json have name,role and status as i needed all these fields in my explorer API.how can i solve this error?this happens whenever we have extra column name in model which is not present in our DB Person.json {
"name": "Person",
"base": "PersistedModel",
"idInjection": true,
"options": {
"validateUpsert": true
},
"properties": {
"name": {
"type": "string"
},
"status": {
"type": "string"
},
"role": {
"type": "string"
}
},
"validations": [],
"relations": {
"products": {
"type": "hasMany",
"model": "Product",
"foreignKey": "personId",
"description": "Returns a list of products associated with a person."
}
},
"methods": {}
} |
|
@ramshgithub AFAICT, your issue is completely unrelated to 'mixinsources' option implemented by this pull request. Please open a new issue in LoopBack's main issue tracker: https://github.com/strongloop/loopback/issues/new |
In continuation with - #79, added support for
mixinSourcesoption.The implementation -
mixinsdirectoryConnect to #79
@bajtos - Can you review?