[Scheduler] Add "combined" www build #16659
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Scheduler module in www is loaded before most of our other infra: too early to conditionally load a separate build in production versus development. Which means we're currently sending both a prod and dev bundle to every single user. We should fix this on the Facebook infra side, but in the meantime, this updates the build script to generate a new type of bundle, `FB_WWW_COMBINED`, which preserves __DEV__ conditions.
|
I should add more context — we don't actually use DEV checks in the Scheduler package currently. We could load the dev bundle in both dev and prod. However, that might not always be the case, so I've opened this PR as a suggestion for how to handle this if it comes up in the future. |
Details of bundled changes.Comparing: f705e2b...e1aefad scheduler
|
acdlite
force-pushed
the
scheduler-www-combined
branch
from
f5e82c5
to
e1aefad
Compare
|
Maybe we should just add a lint check that fails if we ever add a |
trueadm
reviewed
Sep 5, 2019
| bundleType === FB_WWW_PROD || | ||
| bundleType === FB_WWW_PROFILING | ||
| ) { | ||
| if (bundleType === FB_WWW_COMBINED) { |
There was a problem hiding this comment.
Why did you remove the other two bundleTypes here (FB_WWW_DEV and FB_WWW_PROD)?
There was a problem hiding this comment.
Because the purpose of the combined bundle is to replace the dev and prod bundles with a single bundle that includes both code paths
acdlite
added a commit
to acdlite/react
that referenced
this pull request
Sep 11, 2019
Our infra currently doesn't support loading a separate profiling build of Scheduler. Until that's fixed, the recommendation is to load a single build and gate the profiling feature behind a flag. Alternative to facebook#16659
acdlite
added a commit
to acdlite/react
that referenced
this pull request
Sep 11, 2019
Our infra currently doesn't support loading a separate profiling build of Scheduler. Until that's fixed, the recommendation is to load a single build and gate the profiling feature behind a flag. Alternative to facebook#16659
acdlite
added a commit
to acdlite/react
that referenced
this pull request
Sep 11, 2019
Our infra currently doesn't support loading a separate profiling build of Scheduler. Until that's fixed, the recommendation is to load a single build and gate the profiling feature behind a flag. Alternative to facebook#16659
acdlite
added a commit
that referenced
this pull request
Sep 11, 2019
Our infra currently doesn't support loading a separate profiling build of Scheduler. Until that's fixed, the recommendation is to load a single build and gate the profiling feature behind a flag. Alternative to #16659
|
Outdated. Worked around this in www. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Scheduler module in www is loaded before most of our other infra: too early to conditionally load a separate build in production versus development. Which means we're currently sending both a prod and dev bundle to every single user.
We should fix this on the Facebook infra side, but in the meantime, this updates the build script to generate a new type of bundle,
FB_WWW_COMBINED, which preserves DEV conditions.