Skip to content
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

Feature flag to separate Phase 2 features from packages updates targeted as Core fixes/polish. #11016

Closed
youknowriad opened this issue Oct 24, 2018 · 4 comments
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take
Milestone

Comments

@youknowriad
Copy link
Contributor

Post 5.0, this repository will serve for both bug fixes and improvements to phase 1 and phase 2 features.

  • Phase 1 tweaks will be made available to Core as npm package updates
  • Phase 2 features will be available in the plugin and protected behind feature flags

Proposed solution

  • Protect phase2 features behind an environment variable INCLUDE_PHASE2
  • Enable tree-shaking in Core (it should already be the case) to make sure the dead code is removed automatically when bundling the scripts.
  • The build process of the plugin repository should ensure the env variable is set to include the said features.

Thoughts?

@youknowriad youknowriad added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Build Tooling Issues or PRs related to build tooling labels Oct 24, 2018
@youknowriad youknowriad added this to the Future: Phase 2 milestone Oct 24, 2018
@youknowriad youknowriad added the [Type] Task Issues or PRs that have been broken down into an individual action to take label Oct 24, 2018
@noisysocks
Copy link
Member

noisysocks commented Dec 14, 2018

Makes sense to me for WordPress 5.1 features and beyond. I think this is something that we should implement sooner rather than later. Momentum is a fickle thing: if folks are blocked on 5.0.x code freezes then they'll get bored and lose motivation 🙂

We'll need to be very careful when reviewing PRs to test both with the Phase 2 flag on and off. Running the E2E tests with and without the environment variable should help us here. Updating the pull request template to include this as an extra testing step might also help.

How will this impact documentation? We'll need to make sure that npm run docs:build performs tree-shaking so that no Phase 2 APIs are included in the generated docs.

Protect phase2 features behind an environment variable INCLUDE_PHASE2

PHASE=2 maybe? Phase 2 is just the beginning... 😇

@talldan
Copy link
Contributor

talldan commented Dec 14, 2018

I think it could be worth going down a similar route to Calypso's environment specific feature flags:
https://github.com/Automattic/wp-calypso/blob/master/config/production.json#L16-L157

Our environments could be something like:

  • development.js (local dev environments)
  • plugin.js (released to the plugin)
  • core.js (released to core, but still flagged)

Each file contains separate flags for each feature. This provides additional flexibility over a global flag in my opinion and makes it easy to include other environments, or for others forking the project to bootstrap their own environments.

It'd have to be documented well in the contributing guide as working with feature flags does add a bit of friction to contributing.

@youknowriad
Copy link
Contributor Author

Something like Calyspo sound good to me.

Ideally this could even be surfaced in Core later in a UI accessible in a given mode only (debug for instance) to allow people to opt-in to features before the actual release. I won't consider this a critical use-case for v1 of feature flags.

If anyone wants to own this task and experiment with it, please do :)

@talldan
Copy link
Contributor

talldan commented Jan 8, 2019

I did a quick preliminary investigation into this. I haven't quite managed to get dead code elimination working, still investigating that. A couple of things came up:

  • Feature flags might, at some point, need to be accessible within PHP (they are in Calypso from what I understand). I'm not entirely sure how that would work and it's perhaps best not to support it in the first iteration. At least making the config files .json rather than .js would open up future support.
  • Where should the config live? In its own package seems to make the most sense to me, consumers could do something like:
import { getFeatureFlag } from '@wordpress/editor-config';
const isSuperDuperFeatureActive = getFeatureFlag( 'editor/super-duper-feature' );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

3 participants