-
Notifications
You must be signed in to change notification settings - Fork 18
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
New v2 proposal #102
Comments
@ericelliott @thoragio let me know your thoughts on this. |
Perhaps we should call const mergeFeatures = (...featureLists) => featureLists.reduce((a, c) => [...new Set(a.concat(c))]); I'm already envisioning a scenario for universal JS where I'd want to merge features from const browserQuery = ['elephant', 'fish', 'bar', 'cat'];
mergeFeatures(currentFeatures, parseNodeQuery(req && req.query), ['elephant', 'fish', 'bar', 'cat']); Should const NotFoundPage = () => <div>404</div>;
const ChatPage = () => <div>Chat</div>;
const featureOr404 = configureFeature(NotFoundPage);
const Chat = featureOr404('chat', ChatPage);
const myPage = () => (
<Features>
<Chat />
</Features>
); It's fine to move query parsing out of the React components - particularly for easy unit testing, however, the following bits of logic are going to be virtually identical in every app. import parse from 'url-parse';
const url = 'https://domain.com/foo?ft=foo,bar'; // the user is going to have to get the query from the browser API here
const query = parse(url, true);
const initialFeatures = ['faq', 'foo', 'bar'];
const features = activate(initialFeatures, parseQuery(query)); I like the idea of providing both these low-level building blocks (e.g., import { getBrowserQueryFeatures } from '@paralleldrive/react-feature-toggles';
const activeFeatures = activate(initialFeatures, getBrowserQueryFeatures()); Ideally, that function would be universal, and return an empty array in Node. A similar function that takes const { getReqFeatures, getBrowserQueryFeatures } from '@paralleldrive/react-feature-toggles';
const browserFeatures = getBrowserQueryFeatures();
const reqFeatures = getReqFeatures(typeof req === 'object' ? req : undefined);
const activeFeatures = mergeFeatures(initialFeatures, reqFeatures, browserFeatures); Sample implementation: const getReqFeatures = ({ query = {}} = {}) => Object.keys(query); Even this code is a bit too much boilerplate for my taste. This line is particularly obnoxious: const reqFeatures = getReqFeatures(typeof req === 'object' ? req : undefined); Maybe we could wrap that whole thing in a utility? getActiveFeatures(initialFeatures: [...String]) => allTheFeatures: [...String] Provide the primitives, but automate all the things! =) |
I debated about this, but after seeing your use case example, I like
Yup, thanks! Fixed. I like the direction your heading with the query utils and automation, I will mess around with those ideas build on top of them. Thanks! |
@kennylavender This looks great! I like the new way of handling query logic, and the overall way the provider component is simplified. Really nice. |
Great work here. Let's make it happen! |
@ericelliott, I started writing out dream code again with the goal of making the changes we discussed in issues #98 and #99 while also making sure it worked well with the new React context API.
Let me know what you think.
v2 Proposal
This is work in progress
This a proposal that attempts to simplify the API and name components and functions better.
I believe the provider component,
Features
, has more responsibility than needed. I am proposing we simplify the provider component to simply take an array of feature names. My reasoning is that the UI only cares about an array of active feature names. It does not care about things like feature dependencies. As you will see below, removing this logic from the provider component also allows for the rest of this libraries API to become simpler.We will still provide functions that handle query parsing and feature dependencies but now they need to be used to calculate active features name array before it's been passed to the provider component.
Basic Usage
API
Interfaces
Feature
Functions
getEnabledFeatures
([...Feature]) => [...String]
Takes an array of feature objects and returns an array of enabled feature names.
parseQuery
(query = {}) => [...String]
Takes a query object and returns an array of enabled feature names.
mergeFeatures
(...[...String]) => [...String]
Merge feature names without duplicating.
deactivate
([...String], [...String]) => [...String]
Removes feature names
isActive
(featureName = "", features = [...String]) => boolean
Returns true if a feature name is in the array else it returns false.
Components
FeatureToggles
FeatureToggles is a provider component.
props
Feature
Feature
is a consumer component.If the feature is enabled then the activeComponent will render else it renders the inactiveComponent.
Feature takes these props
Feature will pass these props to both the inactiveComponent and the activeComponent
Alternatively, you can use
Feature
as a render prop component by passing a function for the children.HOCs ( Higher Order Components )
withFeatureToggles
({ features = [...String] } = {}) => Component => Component
You can use
withFeatureToggles
to compose your page functionality.Depending on your requirements, you might need something slightly different than the default
withFeatureToggles
. The defaultwithFeatureToggles
should serve as a good example to create your own.configureFeature
(inactiveComponent, feature, activeComponent) => Component
configureFeature
is a higher order component that allows you to configure aFeature
component. configureFeature is auto curried so that you can partially apply the props.Applying query overrides
Query logic has been moved out of the provider component, you should now handle this logic before passing features to
FeatureToggles
The text was updated successfully, but these errors were encountered: