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

fix: Add id to mockCurrentUser #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Alonski
Copy link
Contributor

@Alonski Alonski commented Oct 13, 2022

According to Typescript mockCurrentUser expects a required id.

In Javascript we don't notice this.

Come to think of it... Should this tutorial be in Typescript? Or maybe should we offer a Typescript version?

According to Typescript mockCurrentUser expects a required id.

In Javascript we don't notice this.
@dthyresson
Copy link
Collaborator

@Alonski does the test fail due to the lack of the id?

@dthyresson dthyresson requested a review from cannikin October 21, 2022 14:14
@Alonski
Copy link
Contributor Author

Alonski commented Oct 21, 2022

@dthyresson I'm not sure if the test fails or not. I can check. I just found this while converting to Typescript

@cannikin
Copy link
Member

cannikin commented Nov 7, 2022

Hmm, what if the primary key column isn't named id? Is the Typescript behind there actually smart enough to know that, and expect the correct key there, or is it just hardcoded to id somewhere?

When we originally wrote mockCurrentUser I thought it just took an object and presented that as the return from any currentUser call...not sure why it would need to require a certain set of keys on that object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants