Add version selector

[samselikoff]

Visual treatment

Logic around displaying shas vs. tags

Here's my reasoning behind the logic around showing shas vs. tags:

Current version in Nav

• If current version is latest, and latest points to a tag, then show the tag. Otherwise, show "LATEST".
• If current version is master, and master points to a tag, then show the tag. Otherwise, show "MASTER".
• If current version is a tag, show the tag.

All versions in version selector

• If version is latest, the label is "Latest". If latest has a tag, show tag on right-hand side. Otherwise, show sha.
• If version is master, the label is "master". If master has a tag, show tag on right-hand side. Otherwise, show sha.
• If version is a tag, the label is the tag, and show the sha on the right-hand side.

Happy to discuss if anyone has feedback on this!

Some questions

• We don't have to now but would it be possible to embed/bootstrap versions.json since it's known at build-time? We load it after render so there's a small flicker as we determine how to display the current version in the nav
• Can we make a moduleForAcceptance helper that abstracts the setupApplicationTest and setupMirage (and whatever else we add in the future)? What's the best way to do this in Ember 3?

[dfreeman]

This looks great! 🎉

We don't have to now but would it be possible to embed/bootstrap versions.json since it's known at build-time?

Part of the point of versions.json is that it's shared by (and can be updated independent of) each of the deployed copies of the app, so we can't embed it 🙂
We could look at embedding the version information for the active app during the build, though, which I think would solve the flicker?

Can we make a moduleForAcceptance helper that abstracts the setupApplicationTest and setupMirage (and whatever else we add in the future)? What's the best way to do this in Ember 3?

I saw a suggestion at one point that something like this might become a part of the default blueprint:

// tests/helpers/setup.js
import { setupApplicationTest as qunitSetupApplicationTest } from 'ember-qunit';

export function setupApplicationTest(hooks) {
  qunitSetupApplicationTest(hooks);
  // add custom stuff here
}

And then in all your acceptance tests, you'd do:

import { setupApplicationTest } from 'dummy/tests/helpers/setup';

[dfreeman]

What is this doing?

[samselikoff]

making mirage work when in-repo addons are present :( though I don't understand why it wasn't working, I found this answer in the issues. Let me poke around some more

[dfreeman]

Doesn't seem like a huge deal, I was just curious 🙂

[samselikoff]

fixed it upstream :)

[samselikoff]

We could look at embedding the version information for the active app during the build, though, which I think would solve the flicker?

This would be perfect, we could even make a new Ember Data version model that has an isCurrent flag that's true. Would also let us get rid of the code that inspects the URL to check what the current version is.

[alexlafroscia]

Since other parts of the code access the environment config through the Owner API, this probably should too.

[samselikoff]

yep, I agree we should have consistent style.

Can someone explain to me the difference between importing config and

let config = getOwner(this).resolveRegistration('config:environment');

?

[dfreeman]

To import config, you have to know the name of the host app. Since we're always targeting addons' dummy apps, we could get away with assuming dummy in the import path, but there's nothing stopping someone from picking a different modulePrefix for their dummy app, so using the owner API is safer.

[alexlafroscia]

Can we make a moduleForAcceptance helper that abstracts the setupApplicationTest and setupMirage (and whatever else we add in the future)? What's the best way to do this in Ember 3?

Personally, I kind of like having the explicit repetition of calling both setup methods in each test file. Less guessing about what's going on when reading the tests file.

[samselikoff]

Personally, I kind of like having the explicit repetition of calling both setup methods in each test file. Less guessing about what's going on when reading the tests file.

I guess the question is, when will an acceptance test not need all the hooks? The reason I brought up the issue is because I added an ajax request on boot and I also added Mirage, and so existing acceptance tests no longer worked without Mirage running (the app 404'd).

I think setupApplicationTest is more expressive than 3-4 hooks, which are basically implementation details of how to set up an application test. I'm fine with leaving it how it is for now but I think we should revisit and see if the code feels cleaner with an abstraction.