miragejs / ember-cli-mirage

An Ember Addon to easily add Mirage JS to your Ember app.
http://ember-cli-mirage.com
MIT License
863 stars 439 forks source link

Import server instead of global #1972

Open mehulkar opened 4 years ago

mehulkar commented 4 years ago

Feature request

It would be nice to make the server global importable instead of making it available everywhere.

samselikoff commented 4 years ago

Yep. I'd totally support this if someone wants to work on it, though it is a breaking change.

We could make it either importable (and use module scope, effectively) or register it with the container (and so people could look it up that way. Module scope probably makes more sense.

cah-brian-gantzler commented 4 years ago

I have always used this.server, this being the test instance. where is the server global being talked about? Or does this pertain not to tests but instead app code. In that case wouldnt you create the server in a service and have that service control access?

To do it as an import you would actually have to import a service of some sort that you would call to get access to the server. You cant really import the server instance itself. That being said, thats basically what the container is. I would think using the container would the be correct way to go.

RobbieTheWagner commented 2 years ago

@cah-briangantzler how does this.server work? Trying to use it in beforeEach and it says this.server is not a thing in the 3.0.0 alpha.

cah-brian-gantzler commented 2 years ago

this.server will be the mirage server after setupMirage is executed. Where is your setupMirage in the test?

RobbieTheWagner commented 2 years ago

@cah-briangantzler it does not work. We're doing it in the correct spot. It works with ember-cli-mirage 2.x and does not work with 3.0.0-alpha.3.

module('Acceptance | Directory/Show', function (hooks) {
  let geoEntity!: ModelInstance<GeoEntityModel>;
  let productionRelationship!: ModelInstance<ProductionRelationshipModel>;
  let sentSupplyRelationship!: ModelInstance<SupplyRelationshipModel>;
  let receivedSupplyRelationship!: ModelInstance<SupplyRelationshipModel>;

  setupApplicationTest(hooks);
  setupMirage(hooks);

  hooks.beforeEach(function () {
    this.server.create('ldap-person');

this.server is undefined here.

cah-brian-gantzler commented 2 years ago

I am running

    "ember-cli-mirage": "^3.0.0-alpha.3",

Below is an excerpt from my tests

module('Acceptance | login', function (hooks) {
  setupApplicationTest(hooks);
  setupMirage(hooks);

  test('visiting /login', async function (assert) {
    assert.expect(3);

    let bulletin = this.server.create('bulletin', {
      critical: false,
      bulletinText: 'No IE',
      startDate: new Date().getTime() - 10,
      expirationDate: null,
      location: 'login',
    });

While it is an acceptance test, the setupMirage is the same so I dont think that matters. You can see I use this.server

Here is where this.server is assigned, https://github.com/miragejs/ember-cli-mirage/blob/master/packages/ember-cli-mirage/addon-test-support/setup-mirage.js#L23 Its pretty straightforward. The only reason it could be undefined is that the mirage server was not created, but I think that would give you different issues.

The server is returned from the call to makeServer here https://github.com/miragejs/ember-cli-mirage/blob/master/packages/ember-cli-mirage/addon/start-mirage.js#L44.

Wait, under the 3.0 you need to create your server the same way that it is done in MirageJS (hence the breaking change of 3.0). Thats defined here https://www.ember-cli-mirage.com/docs/advanced/server-configuration Have you converted your config to the new version? This was deprecated and available to the 2.x series.

RobbieTheWagner commented 2 years ago

@cah-briangantzler yeah, we have the new server setup:

import { createServer } from 'miragejs';

export default function (config: ServerConfig<any, any>): any {
  let finalConfig = {
    ...config,
    models: { ...discoverEmberDataModels(), ...config.models },
    serializers: applyEmberDataSerializers(config.serializers),
    routes,
  };

  return createServer(finalConfig);
}

It all works with 2.x and when I update to 3.x there is no this.server

cah-brian-gantzler commented 2 years ago

Would you be able to step through the code and see where the server is failing? Otherwise would need a repo that reproduces the problem for me to step through.

RobbieTheWagner commented 2 years ago

@cah-briangantzler I have stepped through it and it makes no sense to me. It calls setupMirage and when I step through it, it hits the line where it does this.server = but somehow when it returns from the function, it is not set in the tests.

cah-brian-gantzler commented 2 years ago

Is this in setupMirage the test? or is that this somehow not the test? and the server is being set on the wrong context? You will notice here https://github.com/miragejs/ember-cli-mirage/blob/master/packages/ember-cli-mirage/addon-test-support/setup-mirage.js#L14 that setupMirage is really just a beforeEach added to the test suite. this should be the test

RobbieTheWagner commented 2 years ago

@cah-briangantzler I will have to check again, but we are trying to do this.server.create in beforeEach not in a test, so perhaps the beforeEach has a different context?

cah-brian-gantzler commented 2 years ago

put break points in the your before each and the setupMirage before each, the setupMirage before each would HAVE to run first. Are they somehow not running in the correct order?

cah-brian-gantzler commented 2 years ago

Based on the code you pasted before

module('Acceptance | Directory/Show', function (hooks) {
  let geoEntity!: ModelInstance<GeoEntityModel>;
  let productionRelationship!: ModelInstance<ProductionRelationshipModel>;
  let sentSupplyRelationship!: ModelInstance<SupplyRelationshipModel>;
  let receivedSupplyRelationship!: ModelInstance<SupplyRelationshipModel>;

  setupApplicationTest(hooks);
  setupMirage(hooks);

  hooks.beforeEach(function () {
    this.server.create('ldap-person');

SetupApplication takes hooks and adds its own before each, then setupMirage takes hooks and adds its before each, then you add your beforeEach to hooks. They are suppose to run in the order added. Everything looks fine codewise

RobbieTheWagner commented 2 years ago

@cah-briangantzler I stepped through things again, this appears to be a test:

Screen Shot 2022-07-14 at 7 29 55 PM

However, despite it definitely hitting the line where it does this.server = it never sets this.server.

RobbieTheWagner commented 2 years ago

Ah, I finally figured out the issue! this.server = (0, _startMirage.default)(this.owner, options) was actually erroring. Once I moved an import for association to miragejs instead of ember-cli-mirage everything worked. I've been hesitant to move all the imports because they seem to not have types for all the things in miragejs.

cah-brian-gantzler commented 2 years ago

Glad you found the issue.

Since MirageJS was extracted from ember-cli-mirage, been trying to clean up ember-cli-mirage to contain just the things it actually does. The re-export of all the MirageJS Primitives was misleading as to who was responsible for fixing problems.