Avoid non-autofixable ESLint errors in code generated around dbAuth

[Philzen]

While upgrading to v2.0.0 and running the yarn rw lint --fix command, i bumped into one unfixable "error" and warning each. The error makes the command return an error code (=fail) and thus degrade the smoothness of the DX.

Issues:

…/api/src/functions/auth.ts
  105:49  error  'userAttributes' is defined but never used. Allowed unused args must match /^_/u  @typescript-eslint/no-unused-vars

→ fixed in 21c4121

…/web/src/pages/ResetPasswordPage/ResetPasswordPage.tsx
  39:6  warning  React Hook useEffect has missing dependencies: 'resetToken' and 'validateResetToken'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

→ fixed in 6f86667

---

Although it is too late for folks with an existing codebase on the upgrade path to v2.0.0, new RedwoodJS users may benefit from these changes as code generated for dbAuth won't show any ESLint errors.

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 6cf7230. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 14 targets

• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run build
• cli run build
• cli run build
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build

---

Sent with 💌 from NxCloud.

[netlify]

✅ Deploy Preview for redwoodjs-docs canceled.

Name	Link
🔨 Latest commit	6cf7230
🔍 Latest deploy log	https://app.netlify.com/sites/redwoodjs-docs/deploys/62f41a92937c120008389826

[jtoar]

Quick comment about the react-hooks rule. Maybe we should fix that one instead?

[Philzen]

While finalizing this i briefly went through the other files – some of them i never looked at before b/c the stuff i initially wanted to fix in this PR were errors in my project, which doesn't (a) use strict mode and (b) doesn't use webAuthn, which just landed.

Regarding (a) i comitted another quick fix that i found in 94adc37, which adds to #5491
Regarding (b) i found an interesting catch 22 that i didn't want to blindly fix but rather filed this issue so the author can decide: #6171

[Philzen]

Just a CI side-note: is there a reason i cannot see my changes in the netlify preview deploy?

https://deploy-preview-6045--redwoodjs-docs.netlify.app/docs/tutorial/chapter6/comments-schema

[Tobbe]

@Philzen You need to switch from v2.2 to Canary in the dropdown up top

[Philzen]

@Philzen You need to switch from v2.2 to Canary in the dropdown up top

🤦

Duh, it's too hot. Thanks.

[cannikin]

There's some additional variables added to the useEffect on the Reset Password page...did you test this to make sure it didn't introduce any additional re-renders or anything weird?

@dac09 are you okay with adding lint/TS override comments in generated code? I remember someone being particularly adverse to shipping code with those in there, and I'm 60% sure it was you?

[Philzen]

There's some additional variables added to the useEffect on the Reset Password page...did you test this to make sure it didn't introduce any additional re-renders or anything weird?

@cannikin i admit i did not test it but merely followed the advice i was given. Personally, i'd prefer adding // eslint-disable-line react-hooks/exhaustive-deps as it clearly communicates that those dependencies were left out by the developer intentionally – same goes for #6171

@dac09 are you okay with adding lint/TS override comments in generated code? I remember someone being particularly adverse to shipping code with those in there, and I'm 60% sure it was you?

Actually, there already are numerous occasions of these kinds of comments in the code-base, particularly in templates (just make a full-text-search for "no-unused-vars"). I could even find an occasion of // eslint-disable-next-line react-hooks/rules-of-hooks in

redwood/packages/cli/src/commands/setup/i18n/templates/storybook.preview.js.template

Lines 32 to 35 in 1fecb03

	// eslint-disable-next-line react-hooks/rules-of-hooks
	React.useEffect(() => {
	i18n.changeLanguage(context.globals.locale)
	}, [context.globals.locale])

Maybe the core team could re-discuss this and come to a consensus? If comments were deemed OK, i could also immediately fix #6171 as part of this PR.

[Tobbe]

I found this old PR. It had a bunch of merge conflicts that couldn't be automatically resolved by GH, so I fixed those.
Some of the fixes initially proposed by this PR had already been fixed by other PRs, so this one is now a bit smaller than what it was.

There's some additional variables added to the useEffect on the Reset Password page...did you test this to make sure it didn't introduce any additional re-renders or anything weird?

The additions are correct. If for example resetToken changes for some reason, we want the useEffect to re-evaluate. And if it doesn't change (which it most likely won't) the useEffect will only run once, just like with an empty dependency array.

@dac09 are you okay with adding lint/TS override comments in generated code? I remember someone being particularly adverse to shipping code with those in there, and I'm 60% sure it was you?

We're only making existing comments more specific, not adding any new ones. Personally I'd prefer a different solution to the problem, so we could get rid of the comments entirely, but that's for another PR 🙂

[replay-io]

15 replays were recorded for b1543b3.

0 Failed

15 Passed

• useAuth hook, auth redirects checks
• Check that a specific blog post is prerendered
• Check that about is prerendered
• Check that homepage is prerendered
• Check that meta-tags are rendering the correct dynamic data
• Check that you can navigate from home page to specific blog post
• Waterfall prerendering (nested cells)
• RBAC: Admin user should be able to delete contacts
• RBAC: Should not be able to delete contact as non-admin user
• Smoke test with dev server
• Smoke test with rw serve
• Loads Cell mocks when Cell is nested in another story
• Loads Cell Stories
• Loads MDX Stories
• Mocks current user, and updates UI while dev server is running

View test run on Replay ↗︎

[thedavidprice]

@Tobbe did you want this one to go in? I just triggered CI as I assume it was failing due to the flaky tests we resolved last week.

[Tobbe]

@thedavidprice Thanks for the ping!

[thedavidprice]

🚀