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(Playground) #76

Merged
merged 11 commits into from
May 17, 2024
Merged

feat(Playground) #76

merged 11 commits into from
May 17, 2024

Conversation

raxhvl
Copy link
Contributor

@raxhvl raxhvl commented Jan 29, 2024

🪄 Summary

Closes #64 - This feature introduces an interactive playground for quick tinkering with the EthDebug schema.

🎯 Updates

  • A new component added to packages/web/src/components/Playground.tsx
  • Playground tab added in packages/web/src/components/SchemaViewer.tsx
  • By default, the playground shows the first example from each schema if present.
  • Support for 2020-12 draft.
  • Integration with Monaco's native inline errors.
  • Dark mode support.

📦 New Dependencies

  1. @monaco-editor/react@^4.6.0: Monaco IDE component for react.
  2. ajv@^8.12.0: Plugin for JSON schema validation.
  3. [email protected]: Plugin for showing floating error messages.

@raxhvl raxhvl changed the title Feat(Playground) feat(Playground) Jan 29, 2024
@gnidan
Copy link
Member

gnidan commented Jan 30, 2024

Nice job with this! I just tried it out and it works better than I expected. I notice that the ethdebug/format/type playground doesn't report great error messages, but I'm hoping that maybe there's a way to improve that by changing the schema a bit. ethdebug/format/type/elementary seems to work just fine - the errors it gives are quite clear.

I did notice one issue, which is that the toast notifications are a bit wonky... they don't seem to clear properly, like check this screenshot:
Screenshot 2024-01-29 at 21 29 38

Hopefully that'll be pretty easy to sort out... not sure what the requirements should be for toast timing and whatnot, but happy to think of it if you're not sure how it should behave.

Thanks for all your work on this... I'm sure there were parts of it that were frustrating 🙏

@raxhvl
Copy link
Contributor Author

raxhvl commented Jan 30, 2024

Hmm.. This sparked some questions about our toast message approach:

  1. Toast vs. Inline Errors: Do we need both? Inline squiggly feedback during user input seems more intuitive. Also one less npm dependency.
  2. Missing Schema Examples: How should we handle situations where schema examples are unavailable?
  3. Standalone Playground : Should we consider adding a dedicated playground page with schema selection?

Feel free to share your detailed thoughts! We need this tool to be both accurate and user friendly.

This particular error occurs when the schema is not a valid JSON. I think this could very well be inlined.

@gnidan
Copy link
Member

gnidan commented Jan 31, 2024

Hmm.. This sparked some questions about our toast message approach:

  1. Toast vs. Inline Errors: Do we need both? Inline squiggly feedback during user input seems more intuitive. Also one less npm dependency.

Yeah, maybe for simplicity, it makes sense to ditch the toast notifications.

  1. Missing Schema Examples: How should we handle situations where schema examples are unavailable?

Good question... maybe we should just require that all schemas provide examples :)

  1. Standalone Playground : Should we consider adding a dedicated playground page with schema selection?

Sounds like it's worth having one of these :)

Good thoughts!

@raxhvl
Copy link
Contributor Author

raxhvl commented Feb 13, 2024

Ahoy! I have made the following updates:

  • ⛔ Removed react-toastify.
  • ✨ First pass to Monaco itself for JSON validation, if JSON is valid move on to schema validation.
  • ✨ Now reports all errors instead of the first one.
  • ✨ Added JSON Source map to mark errors back to the source key and values.
  • ✨ Auto indent on pasting code. Much UX 🐶.

But... complex schemas still have verbose errors (apart from the useful ones). Could you take a look ? Not sure if this has to do with the schema itself or the AJV options.

Screenshot from 2024-02-13 13-49-48

const ajv = new Ajv({
schemas: Object.values(schemas),
allErrors: true,
strict: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation for AJV options: https://ajv.js.org/options.html

@raxhvl
Copy link
Contributor Author

raxhvl commented Mar 17, 2024

Much cleaner errors:
image

I have made minimal changes to the schema to get this to work.

We could:

  • Make wrapper the root schema instead of type for a cleaner hierarchy.
  • Make a schema file for known type which combines elementary and complex types for now.
classDiagram
   Wrapper <|-- Type
   Wrapper <|-- Reference
   Type <|-- KnownType
   Type <|-- UnkownType
   KnownType <|-- Elementary
   KnownType <|-- Complex
   UnkownType <|-- Base
Loading

But I reckon these changes are mostly cosmetic.

Copy link
Member

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Hey @raxhvl sorry for the delay on reviewing these changes.

Starting here with just a review of the schema changes... looks great. Excellent find on that type wrapper change; I would not have thought of that at all.

I'd like to merge the schema changes separately from the playground stuff - assuming you won't mind, I'm going to cherry-pick and rebase myself to save you the trouble. Happy to show you how I do that sort if thing sometime, if you're curious for the future.

Couple notes, though:

  • You will need to git pull -f on this branch after I do the rebase. [Sorry about this... I like a neat git history and I believe it's worth being mildly annoying.]
  • I noticed your editor is doing automatic formatting for YAML files... this will get annoying, I think, with larger schema change PRs. We should figure out a way to set up prettier or something to run on this repo as a git hook, rather than leaving it to be the editor/IDE's job.

Comment on lines 12 to 19
# Discriminate between reference and type based on presence of `id`
if:
required:
- id
then:
$ref: "schema:ethdebug/format/type/reference"
else:
$ref: "schema:ethdebug/format/type"
Copy link
Member

Choose a reason for hiding this comment

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

Oooh this one is a good catch. Nice find.

Comment on lines 21 to 37
description: Then the object must adhere to exactly one known kind of type
allOf:
- if:
properties:
kind:
$ref: "schema:ethdebug/format/type/elementary#/$defs/Kind"
then:
$ref: "schema:ethdebug/format/type/elementary"
- if:
properties:
kind:
$ref: "schema:ethdebug/format/type/complex#/$defs/Kind"
then:
$ref: "schema:ethdebug/format/type/complex"
Copy link
Member

Choose a reason for hiding this comment

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

Nice :)

(This almost makes me think we should define separate schemas for ethdebug/format/type/kind/elementary etc., rather than reaching into the external schema's $defs. Maybe that's overkill.)

@raxhvl raxhvl marked this pull request as ready for review May 16, 2024 12:34
@raxhvl
Copy link
Contributor Author

raxhvl commented May 17, 2024

@gnidan I have removed package-lock.json.

Copy link
Member

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Bam!

@gnidan gnidan merged commit 9896b3f into ethdebug:main May 17, 2024
4 checks passed
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.

Website should have an interactive playground editor for validating schemas
2 participants