Skip to content

fix(suite-desktop-core): proper name & place for electron-builder hooks#17234

Merged
Lemonexe merged 1 commit intodevelopfrom
fix/electron-builder-scripts-build
Feb 26, 2025
Merged

fix(suite-desktop-core): proper name & place for electron-builder hooks#17234
Lemonexe merged 1 commit intodevelopfrom
fix/electron-builder-scripts-build

Conversation

@Lemonexe
Copy link
Copy Markdown
Contributor

@Lemonexe Lemonexe commented Feb 25, 2025

Description

Fix suite-desktop-core/tsconfig.lib.json, because yarn update-project-references is unstable; produces diff that is not detected by yarn verify-project-references, so it passed CI when I merged it in #17056 😮

  • The diff is in #17231, but on second thoughts, commiting the diff is not right.
  • In our repo, yarn build:lib is always used for building "the main source code" of the given package.
  • But in this case, I want to build just the suite-desktop-core/scripts/ dir
    • electron-builder scripts are totally separate from src/, they don't use any of the workspace imports
  • so it doesn't make sense call it build:lib → rename to build:scripts
    • → does not get picked up by yarn update-project-references, which is correct, because it's a custom task
  • also, if I add all the workspace references, it slows down yarn build:scripts from 5s to 15s on my PC 🐌 🐢 😴

QA

Tested that desktop build works from scratch (all gitignored dirs deleted in suite-desktop) ✔️

Notes

The tsconfig.lib.json passed CI, because yarn verify-project-references throws only on tsconfig.json,
but yarn update-project-references modifies also tsconfig.lib.json.

The script is inconsistent, yarn verify-project-references should throw if there are any changes that would be modified by yarn update-project-references.
That should be fixed, but outside of scope of this PR.

@coderabbitai ignore

@Lemonexe Lemonexe added the no-project This label is used to specify that PR doesn't need to be added to a project label Feb 25, 2025
@Lemonexe Lemonexe force-pushed the fix/electron-builder-scripts-build branch from 0b7fc99 to f5e8510 Compare February 26, 2025 09:40
@Lemonexe Lemonexe marked this pull request as ready for review February 26, 2025 10:02
@Lemonexe Lemonexe requested a review from komret as a code owner February 26, 2025 10:02
"main": "src/app.ts",
"scripts": {
"build:lib": "yarn g:rimraf ./lib && yarn g:tsc --build tsconfig.lib.json",
"build:core": "yarn build:lib && yarn g:rimraf dist && TS_NODE_PROJECT=\"tsconfig.json\" yarn webpack --config ./webpack/core.webpack.config.ts",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗ It was a mistake to include building electron-builder hooks in the build:core, because that's not related to electron-builder at all, it builds JS bundle for electron-main process 🤦
Instead, I moved build:scripts where it logically belongs - as first step of build:app:electron

Ofc it worked incidentally, because build:core gets called before build:app:electron

@Lemonexe Lemonexe merged commit c110dc1 into develop Feb 26, 2025
@Lemonexe Lemonexe deleted the fix/electron-builder-scripts-build branch February 26, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-project This label is used to specify that PR doesn't need to be added to a project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants