New debug and feature flag infrastructure#15043
Conversation
|
☔ The latest upstream changes (presumably #15037) made this pull request unmergeable. Please resolve the merge conflicts. |
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
2 times, most recently
from
7f92588 to
3b45a3d
Compare
package.json
Outdated
| "@glimmer/reference": "^0.22.0", | ||
| "@glimmer/runtime": "^0.22.0", | ||
| "@glimmer/util": "^0.22.0", | ||
| "babel-plugin-minify-dead-code-elimination": "^0.1.4", |
| const WriteFile = require('broccoli-file-creator'); | ||
| const StringReplace = require('broccoli-string-replace'); | ||
| const { RELEASE, DEBUG } = require('./features'); | ||
| const { RELEASE, DEBUG, toConst } = require('./features'); |
There was a problem hiding this comment.
is RELEASE not just !DEBUG?
There was a problem hiding this comment.
nevermind, I see this isn't a bool
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
5 times, most recently
from
b420df0 to
f747349
Compare
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
from
f747349 to
533db41
Compare
chadhietala
changed the title
|
Can you share a tarball of a prod build so we can review? |
rwjblue
left a comment
rwjblue
left a comment
There was a problem hiding this comment.
Can you explain how features get enabled at runtime in both debug and prod builds in the new system (without rebuilding)?
broccoli/packages.js
Outdated
| export default FEATURES; | ||
|
|
||
| ${Object.keys(FEATURES).map((feature) => { | ||
| return `export const ${feature.replace(/-/g, '_').toUpperCase()} = FEATURES["${feature}"];` |
There was a problem hiding this comment.
Isn't this what toConst was for?
broccoli/to-es5.js
Outdated
| options.plugins.push(['minify-dead-code-elimination', { 'optimizeRawSize': true }]); | ||
| } | ||
|
|
||
| // options.plugins.push(['transform-es2015-block-scoping', { 'throwIfClosureRequired': true }]); |
broccoli/to-es5.js
Outdated
| let options = Object.assign({ | ||
| environment: 'developement' | ||
| }, _options); | ||
| options.persist = false; |
There was a problem hiding this comment.
Do we really want to disable cross restart caching?
| }); | ||
|
|
||
| // This is testing that container was passed as an option | ||
| QUnit.test('A deprecated `container` property is appended to every object instantiated from a non-extendable factory, and a fake container is available during instantiation.', function() { |
There was a problem hiding this comment.
This test should not be removed. The container deprecation should still be tested.
There was a problem hiding this comment.
Chad pointed out that this test was already basically skipped just below.
| Element.prototype.webkitMatchesSelector); | ||
|
|
||
| export function matches(el, selector) { | ||
| debugger; |
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
from
533db41 to
7bf379b
Compare
|
This is really cool. I love that we can lean on dead code elimination for more complicated feature flagging combinations! :) |
|
It does not seem possible to enable features at runtime with this at the moment, with this branch (but it does work on canary builds). |
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
2 times, most recently
from
b46540a to
02c475f
Compare
chadhietala
force-pushed
the
the-yak-has-no-hair
branch
from
02c475f to
84ff4a8
Compare
5 participants
This introduces new code stripping infrastructure to lead the way for Project Svelte.