Better feature flag types and add forks route flag #264
Conversation
| featureFlags: { | ||
| routes, | ||
| }, | ||
| } = config as { featureFlags: { routes: { [index: string]: string } } }; // eslint-disable-line no-use-before-define |
There was a problem hiding this comment.
The eslint-disable-line no-use-before-define is needed due to eslint/typescript-eslint-parser#435
config/environment.js
Outdated
| @@ -181,6 +181,7 @@ module.exports = function(environment) { | |||
| dashboard: 'ember_home_page', | |||
| home: 'ember_home_page', | |||
| institutions: 'ember_institutional_landing_page', | |||
There was a problem hiding this comment.
I don't think we have a flag for the route, only the updated flag for the menu item which will probably be institutions_nav_bar. https://github.com/CenterForOpenScience/osf.io/pull/8333/files#diff-758f58f82207e3674b3105d2b3d4ce7aR127
There was a problem hiding this comment.
Indeed. I wonder if we should flag the route though. In this case, the Ember app wouldn't reload, but just go to page not found.
There was a problem hiding this comment.
The thought was that, because the page doesn't exist, if we don't tell anyone about it, there's no reason for anyone to go to the route. It will either need to be on for everyone or no-one, but not showing the menu item is a valid thing to hide to make it easier to sync between teams.
jamescdavis
force-pushed
the
better_feature_flag_types
branch
from
8d0e1f9
to
0b7a536
Compare
Purpose
Improve types for route feature flags and add the missing forks route flag. These changes will restrict references to only known route flags, but you can typecast to string index type to still allow lookup based on a variable (e.g. transition target route name, as demonstrated in
app/router.ts).Summary of Changes
Improve types for route feature flags and add the missing forks route flag.
Side Effects / Testing Notes
Each route flag key will need to be added to
config/environment.d.tsTicket
n/a
Reviewer Checklist
CHANGELOG.md