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

Ensure history doesn't become unreadable when an invalid card is read #26

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

pipcet
Copy link

@pipcet pipcet commented Nov 16, 2024

Hi! This minor fix avoids all history disappearing from the app when an "invalid" card (one for which card_type is undefined) is encountered.

(I've had to make a few minor changes to get things to compile on my machine at all, but those seem unrelated).

@Harry-Chen
Copy link
Contributor

Thanks! But it looks like the commit history is not complete?

@pipcet
Copy link
Author

pipcet commented Nov 17, 2024

Thanks! But it looks like the commit history is not complete?

I'm not entirely sure what you mean. It's a tiny change, yes, but one that is necessary. Using var instead of let here will mean that cardType may become undefined in line 921, and will be used in line 932.

That means the JSON object written to the database won't have a cardType property, since JSON drops properties with undefined values, and the application code relies on the cardType property always to be present, causing the disappearing history bug.

@Harry-Chen
Copy link
Contributor

Thanks! I understand now.

@Harry-Chen Harry-Chen merged commit 01d835e into nfcim:master Nov 18, 2024
2 checks passed
@Harry-Chen
Copy link
Contributor

Your fix is not correct -- it makes card_type always 'Unknown':

var card_type = 'Unknown';
{
  let { card_type, ...detail } = {card_type: 'A'}
}
console.log(card_type) // 'Unknown'!

Harry-Chen added a commit that referenced this pull request Dec 20, 2024
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.

2 participants