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

Split copy-res into two phases during yarn start #26593

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

Johennes
Copy link
Contributor

@Johennes Johennes commented Nov 15, 2023

Requires: #26591

copy-res.ts -w currently runs in parallel with the Webpack build which causes the following problems:

  1. When yarn starting after yarn clean the language file is usually not generated in time for Webpack to process it which breaks the build
  2. Files are being copied even without changes after setting up the watches which triggers another Webpack build

This pull request rearranges copy-res.ts -w so that it only writes files after actual changes being triggered on the watches. This makes it necessary to call the script once without the -w flag before to perform the initial copying which is what's used to perform the initial copying before starting the Webpack build on yarn start. Additionally the initial copy-res is ran in parallel with the module system build and the Jitsi download to gain some more slight performance.

With this change a clean yarn start takes ~21s on my machine. Before, a clean yarn start failed and subsequent yarn start took about the same time.

Unfortunately, even with this change, I'm still often seeing a secondary Webpack build after yarn starting.

2023-11-15 21:54:25.630 [element-js]    2545 modules
2023-11-15 21:54:31.126 [element-js]    2545 modules

There doesn't seem to be any other file generation after the first phase. So I'm unsure what triggers this. It might be similar to what's mentioned in https://github.com/vector-im/element-web/blob/develop/scripts/copy-res.ts#L169 but sleeping doesn't feel like a great solution.


This change is marked as an internal change (Task), so will not be included in the changelog.

@Johennes Johennes requested review from a team as code owners November 15, 2023 20:56
@Johennes Johennes requested review from florianduros and MidhunSureshR and removed request for a team November 15, 2023 20:56
@Johennes Johennes added the T-Task Tasks for the team like planning label Nov 15, 2023
Base automatically changed from johannes/one-watcher-to-rule-them-all to develop November 16, 2023 07:00
@Johennes Johennes enabled auto-merge November 16, 2023 07:01
@Johennes Johennes merged commit fd2f788 into develop Nov 16, 2023
18 checks passed
@Johennes Johennes deleted the johannes/two-phase-copy-res branch November 16, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants