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

Migrate Quote Tool from JavaScript to TypeScript #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Kirollos-Rezkallah
Copy link

@Kirollos-Rezkallah Kirollos-Rezkallah commented Jun 29, 2024

Migrate Quote Tool from JavaScript to TypeScript

Fixes #65

Summary:

This pull request migrates the Quote Tool from JavaScript to TypeScript, enhancing type safety and maintainability. The migration involved converting the main index.js file to index.ts and creating a types.ts file for type definitions. The original index.js and types.js files have been deleted as they are no longer needed.

Details:

  • Migration to TypeScript:
    • Converted index.js to index.ts, ensuring all functionalities are type-safe.
    • Created types.ts to define interfaces and types used in the project.
  • TypeScript Configuration:
    • Updated tsconfig.json to target ES2017 and include necessary libraries (e.g., dom).
    • Enabled strict type-checking options for better code quality.
  • Code Adjustments:
    • Replaced Object.assign with TypeScript-compatible syntax.
    • Ensured compatibility with modern JavaScript features by updating the lib configuration.
    • Fixed issues related to module resolution and type annotations.
  • Deleted Old JavaScript Files:
    • Removed index.js and types.js as they are no longer needed.
  • Testing:
    • Verified successful compilation without errors using tsc.
    • Ensured the output files are correctly generated in the dist directory.

Copy link
Contributor

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

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

you also need to add a build step to the ci

Copy link
Contributor

Choose a reason for hiding this comment

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

jsdocs are missed

import { IconAlignLeft, IconAlignCenter, IconQuote } from "@codexteam/icons";
import { QuoteData, QuoteConfig, TunesMenuConfig } from "./types";

export default class Quote {
Copy link
Contributor

Choose a reason for hiding this comment

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

this class should implement BlockTool

Copy link
Contributor

Choose a reason for hiding this comment

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

its better to rename this file instead of removing, it will be easier to review

Copy link
Contributor

Choose a reason for hiding this comment

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

types are not declared

Copy link
Contributor

Choose a reason for hiding this comment

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

irrelevant change

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.

Migrate to TypeScript
2 participants