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

[TS + Docs] Include potential gql input variables in ListCell's Loading and Success component typing & improve TS docs #11773

Merged
merged 24 commits into from
Dec 16, 2024

Conversation

Philzen
Copy link
Contributor

@Philzen Philzen commented Dec 13, 2024

Although list cells in the simpler examples don't have input variables, they will very well have them in more advanced apps (i.e. only selecting the items that belong to a certain user or given other constraints), so it good to have these input vars available for autocompletion out of the box.

@Philzen Philzen changed the title [TS] Include potential gql input variables in ListCell's Loading and Success component typing [TS + Docs] Include potential gql input variables in ListCell's Loading and Success component typing & improve TS docs Dec 13, 2024
- Fix import order
- add query typing via TypedDocumentNode
- add query variables generic
- fix import ordr
- fix highlighting for some tsx blocks
- add query typing via TypedDocumentNode
- add generic variable typing for Success and Failure components
- fix some tsx highlighting blocks
- add TypedDocumentNode query typing
- add query variable generic
@Philzen
Copy link
Contributor Author

Philzen commented Dec 13, 2024

Phew this was quite a lot of work, but it's done. @Tobbe kindly review at your convenience.

BTW – i'm not sure what prettier is complaining there, i pretty much ran all changed code through my local linter before copying it into the docs.

@Philzen Philzen marked this pull request as ready for review December 13, 2024 19:37
@Tobbe
Copy link
Member

Tobbe commented Dec 13, 2024

@Tobbe kindly review at your convenience.

Will do!

BTW, did you see my DM on the forums?

@Philzen
Copy link
Contributor Author

Philzen commented Dec 13, 2024

BTW, did you see my DM on the forums?

Just saw it two days ago and still digesting it … will answer soon, promised.

@Philzen
Copy link
Contributor Author

Philzen commented Dec 13, 2024

@Tobbe the smoke tests are failing b/c

it('renders Failure successfully', async () => {
expect(() => {
render(<Failure id={42} error={new Error('Oh no')} />)
}).not.toThrow()
})
// When you're ready to test the actual output of your component render
// you could test that, for example, certain text is present:
//
// 1. import { screen } from '@redwoodjs/testing/web'
// 2. Add test: expect(screen.getByText('Hello, world')).toBeInTheDocument()
it('renders Success successfully', async () => {
expect(() => {
render(<Success id={42} blogPosts={standard().blogPosts} />)
}).not.toThrow()
})

these generated tests include an id prop … but the cell doesn't require one 🧐

These IDs appeared in 495f625#diff-61b70e171dde41fe8e245f38f44c78d8ec3354923c8b8b9a9713d1b1f7db74f8 – so i guess idName evaluates to true in the template when it shouldn't.

@Tobbe Tobbe added the release:feature This PR introduces a new feature label Dec 13, 2024
@Tobbe Tobbe added this to the next-release milestone Dec 13, 2024
@Tobbe
Copy link
Member

Tobbe commented Dec 14, 2024

@Tobbe the smoke tests are failing b/c

it('renders Failure successfully', async () => {
expect(() => {
render(<Failure id={42} error={new Error('Oh no')} />)
}).not.toThrow()
})
// When you're ready to test the actual output of your component render
// you could test that, for example, certain text is present:
//
// 1. import { screen } from '@redwoodjs/testing/web'
// 2. Add test: expect(screen.getByText('Hello, world')).toBeInTheDocument()
it('renders Success successfully', async () => {
expect(() => {
render(<Success id={42} blogPosts={standard().blogPosts} />)
}).not.toThrow()
})

these generated tests include an id prop … but the cell doesn't require one 🧐

These IDs appeared in 495f625#diff-61b70e171dde41fe8e245f38f44c78d8ec3354923c8b8b9a9713d1b1f7db74f8 – so i guess idName evaluates to true in the template when it shouldn't.

Fixed in #11779
Thanks for figuring out what the cause was 🕵️‍♂️

@Tobbe Tobbe merged commit 65057dd into redwoodjs:main Dec 16, 2024
49 of 50 checks passed
@Philzen Philzen deleted the patch-23 branch December 16, 2024 15:33
Tobbe added a commit that referenced this pull request Jan 16, 2025
…ng and Success component typing & improve TS docs (#11773)

Although list cells in the simpler examples don't have input variables,
they *will* very well have them in more advanced apps (i.e. only
selecting the items that belong to a certain user or given other
constraints), so it good to have these input vars available for
autocompletion out of the box.

- Adds to #5343 (updating all docs to reflect QUERY annotation with
`TypedDocumentNode`)
- adds to in #11737, as i
realized these now have to be passed in order for the types to still
work.
- Updated documentation for new generated …QueryVariables generic on
Failure and Success components
- docs: fixed some tsx highlighting blocks
- docs: fixed import order to reflect Eslint rules since RW 2

---------

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
@Tobbe Tobbe modified the milestones: next-release, v8.5.0 Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants