1
0
mirror of https://github.com/Unleash/unleash.git synced 2024-11-01 19:07:38 +01:00
unleash.unleash/.github/workflows/gradual-strict-null-checks.yml

74 lines
2.5 KiB
YAML
Raw Normal View History

chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
name: Lower null checks
on:
workflow_dispatch:
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
jobs:
build:
runs-on: ubuntu-latest
env:
MAIN_BRANCH: main
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
strategy:
matrix:
node-version: [18.x]
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
steps:
- name: Checkout current branch
uses: actions/checkout@v4
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
with:
path: current
- name: Checkout main branch
uses: actions/checkout@v4
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
with:
ref: ${{ env.MAIN_BRANCH }}
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
path: main
- name: Use Node.js ${{ matrix.node-version }}
chore(deps): update actions/setup-node action to v4 (#5891) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [actions/setup-node](https://togithub.com/actions/setup-node) | action | major | `v3` -> `v4` | --- ### Release Notes <details> <summary>actions/setup-node (actions/setup-node)</summary> ### [`v4`](https://togithub.com/actions/setup-node/compare/v3...v4) [Compare Source](https://togithub.com/actions/setup-node/compare/v3...v4) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 7pm every weekday,before 5am every weekday" in timezone Europe/Madrid, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Unleash/unleash). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2024-01-17 08:56:41 +01:00
uses: actions/setup-node@v4
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
with:
node-version: 18.x
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
cache: 'yarn'
cache-dependency-path: |
current/yarn.lock
main/yarn.lock
- name: Compare errors if enabling strictNullChecks
env:
URL: ${{ github.event.pull_request.comments_url }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
comment () {
curl -X POST $URL \
-H "Content-Type: application/json" \
-H "Authorization: token $GITHUB_TOKEN" \
--data "{ \"body\": \"${1}\" }"
}
YARN_1="yarn --mutex network --cwd ./current"
YARN_2="yarn --mutex network --cwd ./main"
chore: simplify package scripts (#3736) # Simplify package scripts This PR's purpose is to raise a discussion surrounding our current package scripts. It includes some suggestions that aim to simplify the scripts and hopefully bring a much more straightforward approach to developing and contributing to Unleash. Building (prod) should only happen **explicitly** and when needed. ## Before PR (current behavior) - Clone the project; - Open 2 terminals: One for `unleash` and another for `unleash/frontend`; - On `unleash`: - Run `yarn` (which will also build, for some reason?); - Run `yarn start:dev` to start backend in dev mode (`tsc-watch`); - On `unleash/frontend`: - Run `yarn` (which will also build, for some reason?); - Run `yarn start` to start frontend in dev mode (`vite`); So it seems to me like we build unnecessarily every time we install dependencies. Neither dev scripts need to build the project, as backend uses `tsc-watch` and frontend uses `vite`. I'm unsure why this is the case, as building can take a very long time. ![image](https://github.com/Unleash/unleash/assets/14320932/5ecb7df1-e5b4-4d70-ba7e-97119f5d1116) There's also some complexity in the way we need to split the terminal to `cd` into `frontend` and treat it as a different project. The fact that we have different script names is also confusing (`yarn start`, `yarn start:dev`, etc). ## After PR - Clone the project; - Run `yarn` to install all dependencies; - Run `yarn dev` to get started developing Unleash; Running `yarn` should take care of everything needed to start developing. This includes installing dependencies for frontend as well. It should not build projects if we are not being explicit about it, especially since we don't need to build them at this stage. ![image](https://github.com/Unleash/unleash/assets/14320932/614e42fc-3467-432f-91fc-624b1b35c7c1) Running `yarn dev` should start the project in dev mode. This means running both projects in `dev` mode, which for `backend` means running `tsc-watch` and for `frontend` means running `vite`. Here this PR attempts to provide a better DX by using [concurrently](https://www.npmjs.com/package/concurrently) and [wait-on](https://www.npmjs.com/package/wait-on) - This means both tasks are ran simultaneously, stdout is labeled accordingly, and are stopped together. It also means that `frontend` waits for `backend` to be serving at `4242` before starting, since `frontend` starts pretty much immediately with `vite` and `backend` takes a bit longer. Of course, when the `backend` is hot-reloading you may still find some `ECONNREFUSED`s on `frontend` stdout while it recompiles. ![image](https://github.com/Unleash/unleash/assets/14320932/8bde8ee2-3cad-4e3f-a0db-9eed60cfb04d) No more splitting your terminal and treating `frontend` as a separate project. ## Discussion points Maybe there's a better alternative to `tsc-watch`? I briefly explored some alternatives and while they had a much faster starting speed, hot-reload was sometimes slower. IMO we should aspire to run `src/server-dev.ts` directly and only compile when needed. Running `dev:backend` still serves a version of the frontend (at 4242). **Why? Can we remove that behavior?** I can't imagine a scenario in dev where we wouldn't want to run the latest version of the frontend with `vite`. ~~**Note:** This PR removes all other out-of-scope scripts to focus on this revamp. If we decide to merge it, we should evaluate what other existing scripts we still want to include. May be a good opportunity to clean up unused ones and only include the ones we really use. This includes scripts that our GH actions rely on.~~ **Update:** In an effort to minimize impact surface of this PR and make it a bit more ready for merging: - It updates some docs in https://github.com/Unleash/unleash/pull/3736/commits/2a4ff805e87b65d9c1256effaa189ddcccba15fb and https://github.com/Unleash/unleash/pull/3736/commits/1bbc4882519b5a82e3116f0be255ad24a6f3ce53 to reflect our new simplified flow; - It includes the old package scripts for now in https://github.com/Unleash/unleash/pull/3736/commits/039bc04699ac880e491fd3ce01f9bcd6f97a94b9; - It updates some of our GH actions to reflect the new scripts in https://github.com/Unleash/unleash/pull/3736/commits/7782cb9b12e37ee844507e41ef2b7137eaf55666; Given its current status I'll promote the PR to "ready for review". I still think we should have a second look at our existing scripts and GH actions to see what we really need and/or should adapt, but it should be a team effort so we have a broader context. Maybe on a follow-up PR. Does this require any changes to related projects (e.g. Enterprise)? --------- Co-authored-by: Gastón Fournier <gaston@getunleash.io>
2023-05-12 12:23:22 +02:00
$YARN_1 install --ignore-scripts &> /dev/null && $YARN_1 build:backend --strictNullChecks true 2> .stderr-current > .out-current &
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
pid1=$!
chore: simplify package scripts (#3736) # Simplify package scripts This PR's purpose is to raise a discussion surrounding our current package scripts. It includes some suggestions that aim to simplify the scripts and hopefully bring a much more straightforward approach to developing and contributing to Unleash. Building (prod) should only happen **explicitly** and when needed. ## Before PR (current behavior) - Clone the project; - Open 2 terminals: One for `unleash` and another for `unleash/frontend`; - On `unleash`: - Run `yarn` (which will also build, for some reason?); - Run `yarn start:dev` to start backend in dev mode (`tsc-watch`); - On `unleash/frontend`: - Run `yarn` (which will also build, for some reason?); - Run `yarn start` to start frontend in dev mode (`vite`); So it seems to me like we build unnecessarily every time we install dependencies. Neither dev scripts need to build the project, as backend uses `tsc-watch` and frontend uses `vite`. I'm unsure why this is the case, as building can take a very long time. ![image](https://github.com/Unleash/unleash/assets/14320932/5ecb7df1-e5b4-4d70-ba7e-97119f5d1116) There's also some complexity in the way we need to split the terminal to `cd` into `frontend` and treat it as a different project. The fact that we have different script names is also confusing (`yarn start`, `yarn start:dev`, etc). ## After PR - Clone the project; - Run `yarn` to install all dependencies; - Run `yarn dev` to get started developing Unleash; Running `yarn` should take care of everything needed to start developing. This includes installing dependencies for frontend as well. It should not build projects if we are not being explicit about it, especially since we don't need to build them at this stage. ![image](https://github.com/Unleash/unleash/assets/14320932/614e42fc-3467-432f-91fc-624b1b35c7c1) Running `yarn dev` should start the project in dev mode. This means running both projects in `dev` mode, which for `backend` means running `tsc-watch` and for `frontend` means running `vite`. Here this PR attempts to provide a better DX by using [concurrently](https://www.npmjs.com/package/concurrently) and [wait-on](https://www.npmjs.com/package/wait-on) - This means both tasks are ran simultaneously, stdout is labeled accordingly, and are stopped together. It also means that `frontend` waits for `backend` to be serving at `4242` before starting, since `frontend` starts pretty much immediately with `vite` and `backend` takes a bit longer. Of course, when the `backend` is hot-reloading you may still find some `ECONNREFUSED`s on `frontend` stdout while it recompiles. ![image](https://github.com/Unleash/unleash/assets/14320932/8bde8ee2-3cad-4e3f-a0db-9eed60cfb04d) No more splitting your terminal and treating `frontend` as a separate project. ## Discussion points Maybe there's a better alternative to `tsc-watch`? I briefly explored some alternatives and while they had a much faster starting speed, hot-reload was sometimes slower. IMO we should aspire to run `src/server-dev.ts` directly and only compile when needed. Running `dev:backend` still serves a version of the frontend (at 4242). **Why? Can we remove that behavior?** I can't imagine a scenario in dev where we wouldn't want to run the latest version of the frontend with `vite`. ~~**Note:** This PR removes all other out-of-scope scripts to focus on this revamp. If we decide to merge it, we should evaluate what other existing scripts we still want to include. May be a good opportunity to clean up unused ones and only include the ones we really use. This includes scripts that our GH actions rely on.~~ **Update:** In an effort to minimize impact surface of this PR and make it a bit more ready for merging: - It updates some docs in https://github.com/Unleash/unleash/pull/3736/commits/2a4ff805e87b65d9c1256effaa189ddcccba15fb and https://github.com/Unleash/unleash/pull/3736/commits/1bbc4882519b5a82e3116f0be255ad24a6f3ce53 to reflect our new simplified flow; - It includes the old package scripts for now in https://github.com/Unleash/unleash/pull/3736/commits/039bc04699ac880e491fd3ce01f9bcd6f97a94b9; - It updates some of our GH actions to reflect the new scripts in https://github.com/Unleash/unleash/pull/3736/commits/7782cb9b12e37ee844507e41ef2b7137eaf55666; Given its current status I'll promote the PR to "ready for review". I still think we should have a second look at our existing scripts and GH actions to see what we really need and/or should adapt, but it should be a team effort so we have a broader context. Maybe on a follow-up PR. Does this require any changes to related projects (e.g. Enterprise)? --------- Co-authored-by: Gastón Fournier <gaston@getunleash.io>
2023-05-12 12:23:22 +02:00
$YARN_2 install --ignore-scripts &> /dev/null && $YARN_2 build:backend --strictNullChecks true 2> .stderr-main > .out-main &
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
pid2=$!
# wait for the processes that are expected to fail
set +e
wait $pid1
wait $pid2
set -e
CURRENT=$(grep "Found [0-9]* errors" .out-current | sed 's/Found \(.*\) errors in .* files./\1/')
MAIN=$(grep "Found [0-9]* errors" .out-main | sed 's/Found \(.*\) errors in .* files./\1/')
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
if [ $CURRENT -gt $MAIN ]; then
comment "After enabling [\`strictNullChecks\`](https://www.typescriptlang.org/tsconfig#strictNullChecks) this PR would be **increasing** the number of null check errors from ${MAIN} to ${CURRENT}. <br /> Make sure your branch is up-to-date with ${MAIN_BRANCH} and **check the diff in the console output** to pinpoint the offending files."
diff .out-current .out-main
chore: gradually reduce null-check errors (#3094) ## About the changes In order to move us towards enabling `strictNullChecks` we'd want to have a way of gradually enabling this without having to fix all errors at once, this will force us to start reducing the number of null check issues. This new workflow: 1. [Checks out the current branch and main into 2 different folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23) 2. Uses the **same** script `gradual-strict-null-checks.sh` (from the current branch) [against each folder in parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38) to count the number of errors if `strictNullChecks` was enabled 3. If the number of potential errors in the current branch is higher than the number of potential errors in main [it fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46) As an example, a [new issue was introduced in this PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a) (and then [reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)), so we can test the build failure: https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10 ## Discussion points This could be a non-mandatory check, just advising, or even adding a comment in the PR. It might be good to start with a non-strict check, but at the same time we can decide to make it non-strict if a problem appears In some situations, an additional null check error might require us to fix a bunch of them, increasing the time to deliver. In these cases we can suppress an individual line with `// @ts-ignore: Object is possibly 'null'.` although might defeat the purpose of this workflow
2023-02-14 15:52:21 +01:00
exit 1
else
echo "The PR has $CURRENT null check errors against $MAIN in main. You're good to go!"
fi