improvement(fluid-build): Ignore some tsconfig fields when determining incremental build #23197
Conversation
…g incremental build
github-actions
bot
added
base: main
Fixes several problems causing builds to not be fully incremental. 1. data-object-base had no tests but included test-related tasks. Since no test output was generated, the task was never incremental. Those superfluous tasks have been removed. They can be added back if/when the package is no longer experimental. 2. Added new declarative tasks for `flub check buildversion` and `markdown-magic`. 3. The ai-collab project has `"allowJs": true` in its tsconfig. That setting causes problems with incremental builds (see #23197 for more details). It's removed in this change but if it is truly we can add it back. --------- Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
build-tools/packages/build-tools/src/fluidBuild/tasks/leaf/tscTask.ts
Outdated
Show resolved
Hide resolved
build-tools/packages/build-tools/src/fluidBuild/tasks/taskUtils.ts
Outdated
Show resolved
Hide resolved
|
Reviving this because I still think it's valuable. Need to address the feedback. |
|
Marking draft while I incorporate feedback. |
I switched to this approach. Good idea. |
| // including these settings always fail this check. We special-case the properties in this set -- any differences in | ||
| // those fields are ignored. | ||
| // | ||
| // IMPORTANT: This is a somewhat unsafe action. Technically speaking, we don't know for sure the incremental build |
There was a problem hiding this comment.
On one hand, is anyone going to commonly change allowJs/checkJs and then re-run and incremental build? On the other hand, why is this opt-out instead of opt-in if it is potentially unsafe?
There was a problem hiding this comment.
On one hand, is anyone going to commonly change allowJs/checkJs and then re-run and incremental build?
This isn't quite the situation. Without this change, projects like property-properties and ai-collab have builds that are effectively never cached, regardless of whether people change those projects' settings.
This is because we use the on-disk tsconfig settings to compare with what the tsbuildInfo has. Any project with those settings in their tsconfig is always going to build.
Fortunately these projects are sort of at the edge of the build graph, so most people's day-to-day is not impacted. However, I monitor the cached full build time informally over time, and this is one of the last remaining issues to get to a 100% cached full build. I'd like to get there almost out of spite to the universe at this point. 😆
On the other hand, why is this opt-out instead of opt-in if it is potentially unsafe?
Good point. I remember being proud that I had added an escape hatch, which we've never done with any build-tools changes. But in retrospect I chose a safER option but not the safEST option. I'll make it opt-in.
There was a problem hiding this comment.
I'll make it opt-in.
Done in latest.
When building, fluid-build checks that the cached buildInfo options -- that is, the tsconfig options used to build originally -- match the current tsconfig. If they don't match, then the cache is outdated and a rebuild is needed.
However, there are some tsconfig settings that TypeScript doesn't seem to include in the tsbuildinfo. This makes any builds including these settings always fail this check. We now special-case the properties in this set -- any differences in those fields are ignored. Those fields are:
IMPORTANT: This is a somewhat unsafe action. Technically speaking, we don't know for sure the incremental build status when there is ANY difference between these settings. Thus the safest thing to do is to assume a rebuild is needed. Projects using the ignored settings may exhibit strange incremental build behavior.
For that reason, this behavior must be enabled using the
_FLUID_BUILD_ENABLE_IGNORE_TSC_OPTIONS_environment variable. To enable ignoring these settings, set the environment variable to "1".Projects in the client release group that use these settings: