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

Lint against unnecessary await #1526

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Lint against unnecessary await #1526

wants to merge 3 commits into from

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Aug 28, 2024

This enables linting against unnecessary await in .ts files. It's not enabled on .gts files because unfortunately the type-aware lint rules don't work correctly yet with the gts parser.

This enables linting against unnecessary `await` in `.ts` files. It's not enabled on `.gts` files because unfortunately the type-aware lint rules don't work correctly yet with the gts parser.
Copy link

github-actions bot commented Aug 28, 2024

Host Test Results

       1 files  ±    0         1 suites  ±0   34m 31s ⏱️ + 16m 38s
   656 tests ±    0     654 ✔️  -     1  1 💤 ±0  0 ±0  1 🔥 +1 
1 322 runs  +661  1 316 ✔️ +656  2 💤 +1  2 +2  2 🔥 +2 

For more details on these errors, see this check.

Results for commit fed0cf8. ± Comparison against base commit 0036e88.

♻️ This comment has been updated with latest results.

@habdelra
Copy link
Contributor

habdelra commented Sep 9, 2024

Sees to be some issues with this PR. Seeing a bunch of errors like this after merging upstream changes:

not ok 36 Chrome 128.0 - [undefined ms] - Global error: Uncaught Error: Error loading room Error: Assertion Failed: You attempted to update `_value` on `TrackedStorageImpl`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

`_value` was first used:

- While rendering:
  -top-level
    application
      index
        TestDriver
          OperatorModeContainer
            Modal
              InteractSubmode
                SubmodeLayout
                  ResizablePanelGroup
                    Panel
                      AiAssistantToast
                        (result of a `unknown` helper)
                          (result of a `unknown` helper)
                            (result of a `unknown` helper)
                              this.isVisible

@habdelra
Copy link
Contributor

habdelra commented Sep 10, 2024

Ugh, could this possibly be related to not awaiting?

   ...
not ok 55 Chrome 128.0 - [1391 ms] - Integration | ai-assistant-panel: replacement message should use `created` from the oldest message
    ---
        actual: >
            Wednesday Jan 3, 2024, 12:31 PM Second message body
        expected: >
            Wednesday Jan 3, 2024, 12:30 PM First replacement message body
        stack: >
                at DOMAssertions.includesText (http://localhost:7357/assets/chunk.cd199c2f507c1ac91271.js:64009:10)
                at DOMAssertions.containsText 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants