fix(build): Fix temporary sucrase CI jobs #5021
Merged
Conversation
This file contains hidden or 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
AbhiPrasad
approved these changes
Apr 29, 2022
size-limit report 📦
|
AbhiPrasad
pushed a commit
that referenced
this pull request
May 30, 2022
Until we officially switch over to rullup/sucrase builds, there are extra CI jobs running to test the build changes so far without us having had to switch to them yet. That said, the first attempt at creating such a system had a significant flaw, in that the supposedly-sucrase unit tests were actually running against the normal tsc build output. Further, it was very flaky, but it seems there might occasionally have been some scope bleed between the tsc-built file cache and the sucrase-built one. (It seems unlikely, I know, and I could be imagining it. Regardless, the changes here make it a moot point.) This fixes those unit tests to run against sucrase builds, by adding a script to build with sucrase in the test job itself (and all other sucrase test jobs). It also moves a prior hack (rebuilding in es5 for node 8 tests) from the main test script into the new hacky script, so it can't get missed when pulling these temporary fixes out of the code when we switch. (That move is why our regular unit tests also run that script, to get the prior hack.) Finally, it makes a temporary modification to our base rollup config when in node 8, to enable the just-in-time rollup builds. Disclaimer: This is definitely a hack, and is a little ugly, but it's also coming out in a week, so... ¯\\_(ツ)_/¯ Note: Node 8 sucrase tests are an xfail at this point, until more of the new build pipeline is merged.
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.
Until we officially switch over to rullup/sucrase builds, there are extra CI jobs running to test the build changes so far without us having had to switch to them yet. That said, the first attempt at creating such a system had a significant flaw, in that the supposedly-sucrase unit tests were actually running against the normal tsc build output. Further, it was very flaky, but it seems there might occasionally have been some scope bleed between the tsc-built file cache and the sucrase-built one. (It seems unlikely, I know, and I could be imagining it. Regardless, the changes here make it a moot point.)
This fixes those unit tests to run against sucrase builds, by adding a script to build with sucrase in the test job itself (and all other sucrase test jobs). It also moves a prior hack (rebuilding in es5 for node 8 tests) from the main test script into the new hacky script, so it can't get missed when pulling these temporary fixes out of the code when we switch. (That move is why our regular unit tests also run that script, to get the prior hack.) Finally, it makes a temporary modification to our base rollup config when in node 8, to enable the just-in-time rollup builds.
Disclaimer: This is definitely a hack, and is a little ugly, but it's also coming out in a week, so... ¯\(ツ)/¯
Note: Node 8 sucrase tests are an xfail at this point, until more of the new build pipeline is merged. [ UPDATE: Fixed in #5022. ]