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

feat: transform raw data into FrameMetadata (framegear) #205

Merged
merged 7 commits into from
Mar 7, 2024
Merged

Conversation

cnasc
Copy link
Contributor

@cnasc cnasc commented Feb 29, 2024

What changed? Why?
So far we've been working with the frame data just as a Record<string, string> which causes some unnecessary verbosity and offers the opportunity for typos to lead to runtime errors.

I think it would make sense for framegear to take advantage of the FrameMetadata type that @coinbase/onchainkit provides. After validation, we can pass the results into this converter function and then components can operate on known structured data. This lets us compartmentalize the error prone indexing into the base record into one function.

While I was in there I:

  • updated @coinbase/onchainkit to latest to get the latest type definitions
  • updated the validation and associated test
    • to support state
    • to fix a misleading message
    • to fix a misleadingly named test

Notes to reviewers
Putting this up as draft for now, don't want to change the code too radically to use this just yet. Open to thoughts on this direction

How has it been tested?

@cnasc cnasc marked this pull request as ready for review March 7, 2024 00:05
@cnasc cnasc requested a review from Zizzamia March 7, 2024 00:05
@cnasc
Copy link
Contributor Author

cnasc commented Mar 7, 2024

@Zizzamia this is ready to go in, will start using it once this is merged so we have smaller diffs to review

@Zizzamia Zizzamia merged commit 6ef37ea into main Mar 7, 2024
9 checks passed
@Zizzamia Zizzamia deleted the cjn/schema branch March 7, 2024 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants