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.

Sign up for GitHub

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

Jump to bottom

refactor: Replace JSON.parse with parseJsonWithSchema (@dev-mohit06) #6207

Open

dev-mohit06 wants to merge 4 commits into monkeytypegame:master from dev-mohit06:fix/json-parse-schema

+85 −31

dev-mohit06 commented Jan 23, 2025 •

edited

Loading

Description

Replaces raw JSON parsing with schema-based validation across frontend TypeScript files to improve type safety and error handling.

Scope of Changes

Updated JSON parsing in:
- account.ts
- import-export-settings.ts
- analytics-controller.ts
- local-storage-with-schema.ts
- url-handler.ts
- commandline/lists.ts
- test/wikipedia.ts

Added schema in test/custom-text.ts:

export const customTextDataSchema = z.object({
  text: z.array(z.string()),
  mode: CustomTextModeSchema,
  limit: z.object({ 
    value: z.number(), 
    mode: CustomTextLimitModeSchema 
  }),
  pipeDelimiter: z.boolean(),
});

Benefits

Enhanced runtime type safety
More robust error handling
Consistent JSON parsing approach

Checks

Follows Conventional Commits
Includes GitHub username
Adding quotes? (N/A)
Adding language/theme? (N/A)


          refactor(parsing): Replace JSON.parse with parseJsonWithSchema (@dev-…

797647c

…mohit06)

monkeytypegeorge added the frontend label

fehmer reviewed

View reviewed changes

frontend/src/ts/controllers/analytics-controller.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/controllers/analytics-controller.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/test/custom-text.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/event-handlers/account.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/test/wikipedia.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/test/wikipedia.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/utils/local-storage-with-schema.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/utils/url-handler.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/utils/url-handler.ts Outdated Show resolved Hide resolved

fehmer reviewed

View reviewed changes

frontend/src/ts/utils/url-handler.ts Show resolved Hide resolved

fehmer requested changes

View reviewed changes

Member

fehmer left a comment

Hi @dev-mohit06, nice work. I added some comments for you.

github-actions bot added the waiting for update label

fehmer reviewed

View reviewed changes

frontend/src/ts/utils/url-handler.ts Outdated Show resolved Hide resolved

github-actions bot removed the waiting for update label

dev-mohit06 added 2 commits

January 24, 2025 03:20


          Merge branch 'master' into fix/json-parse-schema

76dc5f3


          refactor: update schema definitions and replace JSON.parse with parse…

475dac5

…WithSchema (@dev-mohit06)

- Updated schema definitions for better clarity and reusability.
- Replaced  with  where applicable.
- Addressed all review comments for better code consistency.

Author

dev-mohit06 commented Jan 24, 2025

Hey @fehmer, could you take a look at the changes?

fehmer requested changes

View reviewed changes

frontend/src/ts/controllers/analytics-controller.ts Outdated Show resolved Hide resolved

frontend/src/ts/controllers/analytics-controller.ts Outdated Show resolved Hide resolved

frontend/src/ts/event-handlers/account.ts Outdated Show resolved Hide resolved

frontend/src/ts/test/wikipedia.ts Outdated Show resolved Hide resolved

frontend/src/ts/utils/url-handler.ts Show resolved Hide resolved

frontend/src/ts/utils/url-handler.ts

-                string | null
-              ];
+              const testSettingsSchema = z.tuple([
+                ModeSchema.nullable(), // Replaces z.union([ModeSchema, z.null()])

Member

fehmer Jan 24, 2025

i don't think this comment adds any value.
Suggestion:

const TestSettingsSchema = z.tuple([
  ModeSchema.nullable(),
  Mode2Schema.nullable(),
  CustomText.CustomTextDataSchema.nullable(),
  z.boolean().nullable(), //punctuation
  z.boolean().nullable(), //numbers
  z.string().nullable(), //language
  DifficultySchema.nullable(),
  z.string().nullable(), //funbox
]);

frontend/src/ts/utils/url-handler.ts Outdated Show resolved Hide resolved

github-actions bot added waiting for update and removed waiting for update labels


          refactor: Update Zod schema naming and type handling (@dev-mohit06)

c422cb9

Author

dev-mohit06 commented Jan 24, 2025

Hey @fehmer, could you take a look at the changes?

dev-mohit06 requested a review from fehmer

January 24, 2025 12:33

github-actions bot added the waiting for review label

fehmer approved these changes

View reviewed changes

Member

fehmer left a comment •

edited

Loading

Hi @dev-mohit06 , changes look good. Please remove the code comment on the unresolved PR comment or replace with the suggested comments.

Thank you for your contribution. Do you want to take the backend part next?

github-actions bot removed the waiting for review label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels