-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add and use loaded features array #563
Conversation
0 ESLint error(s) and 0 ESLint warning(s) found in pull request changed files. |
@dremin Love this idea! Just have a couple questions.
|
The array is valid as soon as the first deferred hook begins to execute. This means we can actually access the array from some hooks before As you can tell, this could be easy to mis-use. I suppose we could address this somewhat by adding a console warning/error when it is accessed too early?
I'm inclined to not document its existence due to the complexity outlined above... |
Considering the need to check if another feature is enabled is a fairly common use case, I'd prefer we provide some documentation on how to safely do so, and this new approach is better than checking Flex config directly for all the reasons you mentioned in the PR. Users of the template will inevitable use this new approach as they look to existing features for examples of how to do it. I'd rather we make the timing concern explicit in our docs versus leaving it open to developers only finding it during testing. If there is a reliable way of also logging a warning / error when it's read too early, that would be a great improvement. Could drastically reduce troubleshooting time if a developer encounters it. |
Fair enough! I have an idea on how to do this, and we can mention the log as part of the doc for the util so that developers are aware of what to expect. |
@trogers-twilio I've added the warning as well as docs, let me know what you think! |
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.
These latest changes look great, thank you Sam!
Summary
Features may need to be aware of certain other features being enabled. For example, pause-recording needs to know if dual-channel-recording is enabled to determine the API to use.
Unfortunately, simply checking if a feature is enabled is not perfect--if a feature has been removed, for example, it is possible that the flex-config lists it as enabled even though the feature does not exist.
This PR adds a simple array to the config helper that is populated by the feature-loader after each feature loads. This provides template features with a list of the actually-loaded features, rather than simply reading the config.
There is a bit of a catch with this, of course--if we haven't finished loading features, the array is incomplete. This means that the array should only be accessed after feature loading is complete.
Checklist