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

Generated: execute yarn lint --fix #575

Merged
merged 2 commits into from
May 3, 2022
Merged

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented May 2, 2022

Generated: execute yarn lint --fix

Follow-up to hotwired/turbo#573.

ESLint support was recently merged and integrated in to the Continuous
Integration checks, but excluded several existing linting violations.

This commit's diff was generated by executing yarn lint --fix.

These violations were discovered when rebasing hotwired/turbo#436.
Once this is passing and merged, these changes will be rebased into that
branch's changeset.

Resolve failing FormSubmissionTest

There is a race condition in the FormSubmissionTest file's coverage
for confirmation. If evaluated quickly enough, the test's assertions
might be checked before the subsequent Turbo Driver navigation occurs,
which would fail the check for the Form Submission's change to the
current path.

This commit adds an additional await expression to wait until the next
turbo:load event to fire, so that the entire navigation can complete
before the assertions are evaluated.

@seanpdoyle seanpdoyle force-pushed the yarn-lint-fix branch 3 times, most recently from 4f400e0 to 346cb12 Compare May 2, 2022 19:29
Follow-up to [hotwired#573][].

ESLint support was recently merged and integrated in to the Continuous
Integration checks, but excluded several existing linting violations.

This commit's diff was generated by executing `yarn lint --fix`.

These violations were discovered when rebasing [hotwired#436][].
Once this is passing and merged, these changes will be rebased into that
branch's changeset.

[hotwired#573]: hotwired#573
[hotwired#436]: https://github.com/hotwired/turbo/pull/436/files
@seanpdoyle seanpdoyle force-pushed the yarn-lint-fix branch 3 times, most recently from ff2bb68 to 938f15d Compare May 2, 2022 20:10
There is a race condition in the `FormSubmissionTest` file's coverage
for confirmation. If evaluated quickly enough, the test's assertions
might be checked before the subsequent Turbo Driver navigation occurs,
which would fail the check for the Form Submission's change to the
current path.

This commit adds an additional `await` expression to wait until the next
[turbo:load][] event to fire, so that the entire navigation can complete
before the assertions are evaluated.

[turbo:load]: https://turbo.hotwired.dev/reference/events
@dhh
Copy link
Member

dhh commented May 3, 2022

Wonder if we could tweak the lint rule here to skip that trailing comma on hashes with just a single pair. Looks bizarre to me 😄

Good to go on the test fix. Probably easier to separate stuff like this into separate PRs going forward.

@seanpdoyle
Copy link
Contributor Author

The latest commits on main have failing CI builds due to the ESLint changes:

I was hoping to restore a passing build with this PR. I feel like it'd be more appropriate to configure ESLint to match preferences with a passing suite.

I was hoping that the ESLint changes could be made in isolation, but the test flakiness addressed in the second commit was also failing CI.

Unfortunately in this case, each change depends on the other change to pass CI.

@dhh
Copy link
Member

dhh commented May 3, 2022

Ah, I see. Could we change the eslint config to drop the trailing commas? Anyway, let's get this passing first.

@dhh dhh closed this May 3, 2022
@dhh dhh reopened this May 3, 2022
@dhh dhh merged commit f67eb3a into hotwired:main May 3, 2022
@seanpdoyle seanpdoyle deleted the yarn-lint-fix branch May 3, 2022 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants