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

chore: clean up RouteParams type test structure #9573

Merged
merged 2 commits into from
Nov 27, 2023
Merged

chore: clean up RouteParams type test structure #9573

merged 2 commits into from
Nov 27, 2023

Conversation

mrazauskas
Copy link
Contributor

This PR removes redundant nesting in a test of the RouteParams utility type.

Comment on lines -93 to +96
context('Glob params in the middle', () => {
test('Multiple Glob route params', () => {
const middleGlob: RouteParams<'/repo/{folders...}/edit'> = {
folders: 'src/lib/auth.js',
}
test('Glob params in the middle', () => {
const middleGlob: RouteParams<'/repo/{folders...}/edit'> = {
folders: 'src/lib/auth.js',
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dac09 We had a conversation about this in #9394 already. I took a better look, seems like this is just a copy paste mistake.

The "Multiple Glob route params" test is defined above:

test('Multiple Glob route params', () => {
const globRoutes: RouteParams<'/from/{fromDate...}/to/{toDate...}'> = {

It tests multiple params indeed, but in the nested case only one param is tested. For me it looks that the nested "Multiple Glob route params" is redundant here. This test case is simply called "Glob params in the middle". Similar to "Starts with Glob route params" case above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah agreed, looks that way to me too :)

Thank you for the PR again!

@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Nov 24, 2023
@dac09 dac09 added this to the chore milestone Nov 24, 2023
@dac09 dac09 enabled auto-merge (squash) November 24, 2023 03:56
@dac09 dac09 disabled auto-merge November 27, 2023 06:32
@dac09 dac09 merged commit ed485ba into redwoodjs:main Nov 27, 2023
26 of 32 checks passed
@mrazauskas mrazauskas deleted the clean-up-type-test-structure branch November 27, 2023 07:33
@dac09 dac09 self-assigned this Nov 29, 2023
jtoar pushed a commit that referenced this pull request Nov 29, 2023
Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com>
dac09 added a commit that referenced this pull request Jan 22, 2024
…nario-multiple-tests

* 'main' of github.com:redwoodjs/redwood: (21 commits)
  fix(deps): update dependency @whatwg-node/server to v0.9.18 (#9602)
  fix(deps): update dependency @apollo/client to v3.8.8 (#9600)
  chore: update yarn.lock
  chore(deps): update dependency @playwright/test to v1.40.1 (#9599)
  chore(deps): update dependency @supabase/supabase-js to v2.39.0 (#9603)
  fix(deps): update dependency @clerk/clerk-sdk-node to v4.12.22 (#9601)
  chore(deps): update dependency @clerk/clerk-react to v4.28.1 (#9598)
  fix(deps): update storybook monorepo to v7.6.2 (#9597)
  RSC: Generate a route manifest (#9592)
  chore(private-set): Wrap profile page in <PrivateSet> instead of Private (#9575)
  add documentation on mocking useParams in component test (#9284)
  Update Typescript to 5.3.2 (#9589)
  RSC: Refactor build process (#9588)
  fix(crwa): clarify docs to avoid issues in yarn modern installs (#9579)
  fix: Prevent `rw graphiql setup` from breaking with encryptSession error (#9582)
  fix: Remove "god code" typos from auth READMEs (#9583)
  Experimental Dockerfile: Fix typo in setup command (#9577)
  fix(streaming-ssr): Fixes running the streaming server using rw serve (#9558)
  9316/update mantine setup (#9388)
  chore: clean up `RouteParams` type test structure (#9573)
  ...
@Josh-Walker-GM Josh-Walker-GM modified the milestones: chore, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants