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

Revert auth provider refactor #6135

Merged
merged 5 commits into from
Aug 9, 2022
Merged

Revert auth provider refactor #6135

merged 5 commits into from
Aug 9, 2022

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Aug 6, 2022

After merging #4880, we started seeing act-related (a React testing thing) warnings show up in web tests that just use render to check if the component renders (see https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/commands/generate/component/templates/test.tsx.template). After trying to solve this (see #6058), we decided that it'd be better to just revert the change for now since we'll be refactoring the auth provider for decoupling auth anyway and can fix the issue then (see #5985, #6029).

So in terms of PR, this PR reverts the following (see commits 37d686a, 1e0cfef):

And slightly changes the following (commits ac95080, 14ddc26):

@jtoar jtoar added the release:fix This PR is a fix label Aug 6, 2022
@jtoar jtoar requested review from Tobbe and dac09 August 6, 2022 07:14
@jtoar jtoar self-assigned this Aug 6, 2022
@nx-cloud
Copy link

nx-cloud bot commented Aug 6, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b33ef7e. 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

Sent with 💌 from NxCloud.

@netlify
Copy link

netlify bot commented Aug 6, 2022

Deploy Preview for redwoodjs-docs canceled.

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

@jtoar
Copy link
Contributor Author

jtoar commented Aug 6, 2022

I ran build:test-project --rebuild-fixture to update the test project fixture and there's two things that may need some attention:

  • The rebuild process deletes the comments in schema.prisma. Should we keep them? @dthyresson are they used in tests?
  • The users.scenario.ts file gets deleted, but seems like it shouldn't be

@jtoar
Copy link
Contributor Author

jtoar commented Aug 6, 2022

@zackdotcomputer tagging you here in case you see this one cause I don't want you to feel bad. Not your fault at all—just the wrong time for us, and we couldn't find a fix we were happy with. We do have a real fix in the works, but it'll take more time, and we're a little overdue on a release, so just trying to be pragmatic here. Thank you for the work you did, it's informing our decisions for decoupling auth and teaching us what to look out for!

@Tobbe
Copy link
Member

Tobbe commented Aug 6, 2022

@zackdotcomputer You changes live on in spirit here: #5985 (It'd actually be great if you could take a look at the Clerk parts of that PR) + here: https://github.com/Tobbe/rw-decouple-auth-demo/blob/main/web/src/auth.tsx

@Tobbe Tobbe self-assigned this Aug 6, 2022
@dac09
Copy link
Contributor

dac09 commented Aug 8, 2022

Hey Dom, not sure i can anything more to reviewing this PR - you seem to have a good handle on what changes are required here.

But the setup auth clerk command seems to work (I don't have clerk creds to do a full e2e).

I think its important we merge this in ASAP so we can continue shipping!

Copy link
Member

Tobbe commented Aug 8, 2022

Thanks for the kick in the butt Danny 😄 I do have Clerk creds, so I ment to do a full test, but I didn't get to it yet

Copy link
Contributor

dac09 commented Aug 8, 2022

Haha not a kick in the butt - just a gentle nudge to the shoulder 😉

@jtoar jtoar mentioned this pull request Aug 8, 2022
18 tasks
@Tobbe
Copy link
Member

Tobbe commented Aug 9, 2022

I just confirmed it worked in a new project I created with the code in this PR. Could generate the auth setup and login using clerk's dialog and my clerk credentials 👍

@jtoar jtoar added the fixture-ok Override the test project fixture check label Aug 9, 2022
@jtoar jtoar enabled auto-merge (squash) August 9, 2022 14:24
@jtoar jtoar merged commit e40ba15 into main Aug 9, 2022
@jtoar jtoar deleted the ds-revert-auth-provider-refactor branch August 9, 2022 14:55
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Aug 9, 2022
dac09 added a commit to dac09/redwood that referenced this pull request Aug 9, 2022
…ac09/redwood into fix/user-scenarios-not-being-generated

* 'fix/user-scenarios-not-being-generated' of github.com:dac09/redwood:
  Amend import order in auth templates according to ESLint rules introduced in v2.0 (redwoodjs#6059)
  docs(tutorial): Adds more TS hints in the toturial (redwoodjs#6115)
  Revert auth provider refactor (redwoodjs#6135)
dac09 added a commit to dac09/redwood that referenced this pull request Aug 10, 2022
…-types-on-setup-auth

* 'main' of github.com:redwoodjs/redwood: (33 commits)
  fix(generators): Dont set tests=false in sdl generators by default (redwoodjs#6195)
  Harmonize optional chaining across all `hasRole` flavors & reduce duplication, avoiding errors on empty currentUser (redwoodjs#6159)
  Amend import order in auth templates according to ESLint rules introduced in v2.0 (redwoodjs#6059)
  docs(tutorial): Adds more TS hints in the toturial (redwoodjs#6115)
  Revert auth provider refactor (redwoodjs#6135)
  fix(deps): update docusaurus monorepo to v2.0.1 (redwoodjs#6176)
  fix(deps): update typescript-eslint monorepo to v5.33.0 (redwoodjs#6192)
  fix(deps): update dependency yargs-parser to v21.1.1 (redwoodjs#6191)
  fix(deps): update dependency fastify to v4.4.0 (redwoodjs#6190)
  fix(deps): update dependency @testing-library/user-event to v14.4.2 (redwoodjs#6189)
  chore(deps): update dependency @types/vscode to v1.70.0 (redwoodjs#6188)
  chore(deps): update dependency @types/prettier to v2.7.0 (redwoodjs#6187)
  fix(deps): update dependency systeminformation to v5.12.4 (redwoodjs#6186)
  chore(deps): update dependency @clerk/types to v2.21.0 (redwoodjs#6179)
  chore(deps): update dependency lerna to v5.4.0 (redwoodjs#6180)
  fix(deps): update prisma monorepo to v4.1.1 (redwoodjs#6181)
  chore(deps): update dependency @clerk/clerk-js to v3.17.0 (redwoodjs#6178)
  fix(deps): update storybook monorepo to v6.5.10 (redwoodjs#6177)
  fix(deps): update dependency @actions/core to v1.9.1 (redwoodjs#6175)
  chore(deps): update dependency magic-sdk to v9 (redwoodjs#6138)
  ...
@jtoar jtoar removed this from the next-release milestone Sep 2, 2022
@jtoar jtoar added this to the v3.0.0 milestone Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixture-ok Override the test project fixture check release:fix This PR is a fix
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants