Embroider: DBs are not populated
gilest opened this issue · 7 comments
When I enable the staticAddonTrees: true
option in the Embroider config it seems to break the mirage database.
Specifically the detection of ember-data models.
With the above option enabled the mirage server.db
is missing all the regular ember-data models in my app and only has entries for models I defined a mirage factory for.
Asked in #ec-mirage
on the Ember discord and another user mentioned they had the same issue.
I made a dummy repo with a failing test demonstrating the issue. Disabling staticAddonTrees
will cause this test to pass.
Tracked this down to _hasEmberData.
function _hasEmberData() {
let matchRegex = /^ember-data/i;
return !!(Object.keys(requirejs.entries).find(e => !!e.match(matchRegex)));
}
When built with staticAddonTrees: true
there are far fewer modules returned by requirejs.entries
. The only one referencing ember-data
in the demonstration app is @ember-data/model/-private
. Is this an accurate module to check for in _hasEmberData
?
If so then the regex could be adjusted slightly.
let matchRegex = /^@?ember-data/i;
So the problem appears to be the requirejs.entries
. Yes modifying the regex here will get you a little farther, but in getDSModels
there is another requirejs.entries
in getDsModels
and thats where the real problem will be.
As I see it in my testing, requirejs.entries
contains all the modules available. But when you do the staticAddonTrees
it appears to ONLY contain the modules within the current addon/app that is is being executed in. This means that ember-cli-mirage will have no access to modules outside of itself.
I looked into using the registry but getOwner(this).factoryFor()
doesnt provide a list. You have to know the model name before hand. If there were a getOwner(this).factoriesForType('model')
provided I think this could be done without being too brittle. Any changes to how requirejs handles things will break ember-cli-mirage which is why we have the current problem. We should put in a RFC for that. However I think the need would be only for a very few addons, so not sure if they would do it or not.
In the meantime I do have a solution. First, since MirageJS has been extracted out, and has a very different server create, changes have been made to ember-cli-mirage to make its server create a lot more like the mirageJS one. This is avail today (it is not the default blueprint. I wish it was). You can find that documented here https://www.ember-cli-mirage.com/docs/advanced/server-configuration and I will give below. We would need to change the configuration to this
import { createServer, discoverEmberDataModels } from "ember-cli-mirage";
export function makeServer(config) {
let finalConfig = {
...config,
models: { ...discoverEmberDataModels(), ...config.models },
routes,
};
return createServer(finalConfig);
}
function routes() {
// this.namespace = '/api'
// this.resource('user')
}
We now have a more direct access to the function discoverEmberDataModels()
. I would have to make a PR to ember-cli-mirage from here but if we changed the above configuration to this
import { createServer, discoverEmberDataModels } from "ember-cli-mirage";
export function makeServer(config) {
let finalConfig = {
...config,
models: { ...discoverEmberDataModels(requirejs.entries), ...config.models },
routes,
};
return createServer(finalConfig);
}
function routes() {
// this.namespace = '/api'
// this.resource('user')
}
Since the requirejs.entries
is now being executed within the app, it will contain the models defined in the app. But the use of requirejs.entries
here just moves the brittleness into the end users config. Not sure if Sam will be ok with that, but its the only solution I have found so far. I am not an expert in the ember build process so if someone sees a better solution please let me know.
Just in case someone thinks this, we can not export a function from ember-cli-mirage to encapsulate the requirejs.entries
because that will make it exectute within the ember-cli-mirage space and get the wrong results (which is exactly what is happening now :( )
If someone wants to check my research, or give an alternate solution for getting to a list of models please let me know.
In your dummy repo, if you change the package.json to use my repo
"ember-cli-mirage": "git+https://github.com/bgantzler/ember-cli-mirage#staticaddontrees",
and change your mirage/config.js to
/* global requirejs */
import { createServer, discoverEmberDataModels } from 'ember-cli-mirage';
export function makeServer(config) {
let finalConfig = {
...config,
models: { ...discoverEmberDataModels({moduleMap: requirejs.entries}), ...config.models },
routes
};
return createServer(finalConfig)
}
function routes() {
}
You should find it now works.
Exposing the requirejs.entries
though is prob not a long term solution. Just the only thing I could figure out at the moment.
@cah-briangantzler Thanks for the detailed investigations and response.
You're right – the proposed solution works in the example app (branch here)! 👏
One thing I'd like to point out – ember-cli-mirage is able to discover ember-data models in the example app if _hasEmberData
returns true.
The module list is much less when running under Embroider but it still includes the app's own modules such as ember-data models.
For example see this diff which compares the output of requirejs.entries
in the example app when building without Embroider (on the left) and with Embrdoier(with staticAddonTrees: true
). Note that the app (embroider-mirage
) modules are available in both scenarios, but there is a limited number of other modules available eg. the ember-data
ones.
I could have sworn when I step through the models were not in the list. But if they indeed are, then just changing the regex as you purposed is a much better solution. I will try that again then and let you know
Weird, I dont know how I missed it. Yea the models are there. That can be proved by removing the requirejs.entries being passed to discover models method and it all still works.
So you were correct, it was just the one regex. I have updated my branch to check both regex rather than change the regex as in ember 2.18, adding the @ to the regex would not have worked. I am not sure how far back ember-cli-mirage is compatible but that one line doesnt hurt.
Tested the example app again but this time with no config modifications (branch here). Tests pass ✅
I have updated my branch to check both regex rather than change the regex as in ember 2.18, adding the @ to the regex would not have worked. I am not sure how far back ember-cli-mirage is compatible but that one line doesnt hurt.
Great – I wasn't sure about version support of the different module names.
I think we'll be fine though, as the oldest version being tested in ember-try is 3.20.0
.