-
Notifications
You must be signed in to change notification settings - Fork 440
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
Provided a makeServer function on config.js to allow more access to the mirageJS configurations #1994
Conversation
a5556fd
to
95fe7d4
Compare
Ok – I think this is what we should do: /*
The named `makeServer` function export gives you a lower-level way to hook
into how your Ember app instantiates your Mirage JS server.
Typically, the `/mirage/config.js` file contains a single default export which
is a function defining all your Mirage route handlers. Ember CLI Mirage then
uses this function, along with all the other modules you've defined in
`mirage/models`, `mirage/fixtures`, mirage/factories`, and
`mirage/serializers`, to create your Mirage JS server when your app boots up
in development and testing.
You can now opt in to having more control over exactly how your Mirage server
is instantiated, as well as the ability to use imports directly from the
`miragejs` npm package, by exporting a single named function called
`makeServer` instead.
`makeServer` receives a single argument called `config`, which contains all
the factory/fixture/serializer/model modules that exist in your project's
`/mirage` directory. This saves you from having to import each module
explicitly and then pass it into your Mirage server, just like you're used to
with the default setup.
The `config` argument maps exactly to everything inside of your `/mirage`
directory - notably, it does not contain the autogenerated Mirage model
definitions derived from your Ember Data models, which is an important feature
of Ember CLI Mirage that is enabled by default. To replicate this behavior
when using `makeServer`, we have provided an additional import called
`discoverEmberDataModels` from the `ember-cli-mirage` package that you can use
to augment your config with these models so that your Mirage schema is
automatically inferred from your host application's Ember Data models and
relationships. The snippet below shows how to do this. Note that the order
here matters if you also have models defined in your `/mirage/models`
directory, as those model definitions would "win" in the event of a conflict
with the ones autodiscovered from Ember Data. (However, most of time if you
are inferring your Mirage schema from Ember Data, you shouldn't need to define
additional models.)
Finally, your route handlers just need to be passed to the `routes()` key in
your Mirage config. You can do this inline, or you can make them a separate
function, and organize that function however you choose.
The only thing needed to enable this behavior is that you delete the default
function export and instead export a single named function called
`makeServer`. You should also add `miragejs` to your project's dependencies in
your `package.json` file, since you are now importing directly from it. Note
that this gives you the added benefit of being able to upgrade `miragejs`
independently of `ember-cli-mirage`.
Eventually, `ember-cli-mirage` will shed its re-exports of everythign from
`miragejs', and become a small wrapper library delegating the rest of the work
to `miragejs`. This will help align the Ember Mirage users with the rest of
the Mirage JS community.
*/
// Example with inline routes
import { createServer } from "miragejs";
import { discoverEmberDataModels } from "ember-cli-mirage";
export function makeServer(config) {
let finalConfig = {
...config,
models: { ...discoverEmberDataModels(), ...config.models },
routes() {
// this.namespace = '/api'
// this.resource('user')
},
};
return createServer(finalConfig);
}
// Example with routes in an external function
import { createServer } from "miragejs";
import { 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')
} |
Basically just change Finally, let's undo the changes to the blueprint. We have some more work to do to deprecate the |
Yep, I forgot the mirageJS doesnt carry through, which is why all the files for import export like belongsTo and hasMany for example. Until these are shed, could we add one more to the pile and just import/export createServer to allow the blueprint to promote the newer way for new installs now? (just re-export createServer instead of
Didnt think about this syntax, yea if you want the merge to happen here no problem.
[Deleted incorrect comment] |
actually createServer already is exported in index.js
|
Given
Which should be be passing into makeServer, routes or baseConfig? Which is the preferred way? |
Sure, that's fine 👍
|
Your whole comments above with examples are great, where should I put that? In the docs? Maybe a new nav called "server configuration" |
All the test are passing, but getting a linting error
Its complaining about the
|
Ok fixed the linting When trying to check the documentation, when running
added ember-fetch to the package.json, then got the following
Needless to say, have not successfully run the demo app This PR should have the suggested changes with the exception the blueprint is still altered but is instead
To account for the correct import |
Ok, are you able to run the docs app on (Am I correct in remembering you had trouble getting it running recently?) |
Yes it would not start due to an error you fixed (fabd388), once it started I thought I was good, but when navigating to the url I now get the above errors. Yes, I get the same missing fetch on master, add ember-fetch and then get the next error, did not continue fixing errors. I was looking at the tests directory and there are not really tests, however I think I could write an integration for a fake component that gets data from the a mirage source. If I then use the new config to do that, I could remove the test app 03. I assume you would like me to look into that given test-03 was a concern earlier? |
I think test-03 is fine bc we can delete it if it ever becomes a problem. We should figure out why the demo app is not working for you though eh? You're saying, you can run |
I am on the master branch,
Adding
Do not know what this error is so couldnt continue I would think if you cloned the repo you would get the same thing |
261510c
to
f6543f8
Compare
Noticed that there were some dependency updates on master. Rebased and now the docx app runs again. Did have a slight missing entry. Fixed that. PR should be good to go now. |
…he mirageJS configurations
f6543f8
to
8b99441
Compare
Hopefully we have gotten past all the hurdles and we can get this merged so I can get the Original PR that started all this finished 👍 |
let server; | ||
if (makeServer) { | ||
server = makeServer(options); | ||
if (typeof location !== 'undefined' && location.search.indexOf('mirageLogging') !== -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this guy supposed to be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because when we call makeServer we are by passing the ember-cli-mirage Server class extend, and this code was in that Server class. Without it here the mirage logging check box will not work. I found this when i rebased my application to use a make server and I was running tests.
When the old way is deprecated and then removed, the Server class in ember-cli-mirage will also be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah the qunit toggle. Gotcha, thanks!
Only last q is, should we change the default blueprint? I kinda think we should leave it alone since |
In order to reduce the foot print of ember-cli-mirage at some point you would have to deprecate and make the makeServer function mandatory. If we change the blue print now, fewer people will be affected by the change, they just ignore the boiler plate code and change the route function as before, if they need the more advanced its setup for them and ready to go. If I were a new user, I would have liked that you saved me an eventual change down the road. If you dont plan on deprecating and removing then old way, then I would say yes, maybe not change the blueprint. So its up to you, I can change it back. |
Definitely plan on deprecating + removing but, I think you might be underestimating how confusing this would be for someone new to the addon, especially since it's not documented anywhere on the site yet, and all the examples show something different when referencing the Thanks for keeping pushing on this + sorry it's taken me a while to go back and forth! |
Maybe you might to add some comments above the make function similar to what type script does for boilerplate code. // DO NOT DELETE: this is how TypeScript knows how to look up your controllers.
declare module '@ember/controller' {
interface Registry {
'application': Application;
}
} I like it at the top, because I would move the routes to another file, but we could move it to the bottom maybe? I didnt put the enitre comments you added above in the blueprint, that seems a bit much, but we could put the whole thing there if we moved it to below the routes function. Or maybe an abbreviated set of those comments |
K, Ill change it back then, give me a few |
Yeah I think we could come up with some way to document it via comments but it feels like rushing it, I say we just wait until we can actually document this. |
I thought about this and Ill add just these points. As a new user, if I cant find what I need in ember-cli-mirage, I might go to the mirageJs website and there all the examples talk about configing the server a completely different way, if I saw the make server function, I would see some common code between MirageJs and ember-cli-mirage to help bridge that gap. Second, the followup will be the work I did for reverse engineering the serializers much the way you do with models. I would like it to be part of this addon instead of its own but either way the point is the same, it will only be supported under the makeServer option (why we started down this road), which means in order to use the new feature, they would be forced to change the config rather than add an import and a property to the options config (new users only of course, cause thats where the blueprint comes into play) I started looking through the examples in the guides and under the functions tab the first example is this
It says config.js, but clearly is not showing the entire config, just the part your interested it, so in overview when it lists this
the same would imply, you are talking about this function, not showing the entire config. So if we went back to export default unnamed, I dont think any of the current docs would be misleading. (By doing so they do have to infer later it doesnt have to be export default, you can name and use. not so much an issue. So some food for thought. Ill change the blue print back, but easy enough to put back in later |
Understood and appreciate the thoughts. I think I just want to do a bit of planning so we can tie up all these loose ends together. Ryan and I are going to be shipping Mirage v1 in a month from now and I want to get ember-cli-mirage users basically reading those docs and get rid of the majority of these ones. I think that will be a good time to revisit this. In the mean time we can document in a guide on this site. |
Ok, I think this is good to go? |
I agree, other than the blueprint revert that wouldnt matter, I have converted my company app over to use the branch and all the tests worked as expected (thats where i found the qunit thing so its a good thing I did that) |
For the next question though, are you open to adding the serializer generation in this addon or do I need to make another (which I can now do because of this PR) |
Any idea when this may be merged and I can move onto the serializer generation? |
Sorry I think I was waiting for tests to pass last time! Can you open new issue laying out the serializer APIs you need so we can discuss there? |
First PR for discussion in #1989
There is really only a small bit of code in a a few files.
addon/ember-data.js
addon/index.js
addon/start-mirage.js
app/initializers/ember-cli-mirage.js
All the other files are test related with most as a clone of test project 1 into test project 3 to show makeServer working.
mirage/config.js can now be written as