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

Makes validateWith async and a new validateWithSync synchronous #7681

Merged
merged 10 commits into from
Mar 23, 2023

Conversation

cannikin
Copy link
Member

@cannikin cannikin commented Feb 23, 2023

Right now the validateWith() function doesn't work with functions that return promises, it blows up the API server. This PR updates the function to, by default, work with promises, and creates a new validateWithSync() which duplicates the old behavior of working with non-async functions only. The naming convention follows those of other React packages like fs.

Unfortunately, because it changes how the default function works, this is a backwards incompatible change and will need to be released in a major version. Whenever 5.0 is ready!

Todo:

  • Docs
  • Tests

@cannikin cannikin added the release:breaking This PR is a breaking change label Feb 23, 2023
@cannikin cannikin self-assigned this Feb 23, 2023
@replay-io
Copy link

replay-io bot commented Feb 23, 2023

16 replays were recorded for 8ebe600.

image 0 Failed
image 16 Passed
    requireAuth graphql checks
          ```
          locator.waitFor: Target closed
          =========================== logs ===========================
          waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <li><a href=https://app.replay.io/recording/94c3f9f3-4501-4028-a922-410f04b1e597>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/40a17cf3-7dc8-4246-8561-160d77aa244b>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/bfa749ae-3306-41df-b2a0-c8ae5aaa9d9e>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/2e08f996-4533-478f-8924-32d29c4ef917>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/23528ef4-5353-457f-b493-6cc371827eaf>Check that meta-tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/b4eef66e-3dcf-4b55-908f-fcffabed411a>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/855a0d21-28e8-492d-93c9-2b0125bac535>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/1c853b24-e6e6-430e-bbbb-b2eee7578ff7>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/93ed2b0d-977c-4378-beaa-be0637c4f563>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/850f2293-b0f7-4e95-9fe6-fb1d84436409>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/9fb44b84-009b-4ebb-8bde-1dbdda8d65fa>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/1d96f89d-5572-47aa-a3a7-6f6524602a0a>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/a4f82340-de50-4de9-9068-a99c9c7c3acc>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/1fc5e950-2057-4f6e-98ff-63569d2141a9>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/e6a6331e-569d-4a93-9456-fbaefb286c6a>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@jtoar
Copy link
Contributor

jtoar commented Mar 21, 2023

@cannikin I think the codemod just needs to change all instances of validateWith to validateWithSync right?

@cannikin
Copy link
Member Author

Yep that should be it!

@cannikin cannikin marked this pull request as ready for review March 23, 2023 18:11
@cannikin
Copy link
Member Author

@jtoar Looks like this is good to go, feel free to merge whenever you're ready!

@jtoar jtoar merged commit 9bdf3b0 into main Mar 23, 2023
@jtoar jtoar deleted the rc-validate-with-async branch March 23, 2023 20:58
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Mar 23, 2023
@jtoar jtoar modified the milestones: next-release, v5.0.0 Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants