Migrate to TypeScript and update project configuration, issue #72 #76

amriksingh0786 · 2025-02-17T10:06:00Z

Migrate to TypeScript and update project configuration #76

neSpecc · 2025-02-17T10:29:53Z

src/index.ts

 * Build styles
 */
-import './index.css';
+import "./index.css";


please, use single quotes

still actual

neSpecc · 2025-02-17T10:31:35Z

src/index.ts

+// Add these interfaces at the top of the file
+interface HTMLPasteEventDetail {
+  type: "html";
+  data: HTMLElement;
+}
+
+interface FilePasteEventDetail {
+  type: "file";
+  file: File;
+}
+
+interface PatternPasteEventDetail {
+  type: "pattern";
+  data: string;
+}


these interfaces are redundant since editor.js package already includes these types

neSpecc · 2025-02-17T10:31:58Z

src/index.ts

+}
+
 export default class SimpleImage {
+  private api: any;


specify exact type instead of any

neSpecc · 2025-02-17T10:32:43Z

src/index.ts

+  data: string;
+}
+
 export default class SimpleImage {


class should implement BlockTool

src/index.ts

neSpecc · 2025-02-17T10:33:58Z

src/index.ts

   */
-  constructor({ data, config, api, readOnly }) {
+  constructor({
+    data = {} as Partial<SimpleImageData>,


it is the logic change, please, explain it

This change improves type safety by:

Ensuring data is always an object (defaulting to empty object if not provided)

Explicitly marking data as a partial version of SimpleImageData type, meaning not all properties are required

actually data can't be an empty object. As well and partial

neSpecc · 2025-02-17T10:34:41Z

src/index.ts

    return Object.assign(this.data, {
-      url: image.src,
-      caption: caption.innerHTML,
+      url: image?.src || "",


image and caption cant be undefined, so this logic never be used

neSpecc · 2025-02-17T10:35:56Z

src/index.ts

+    if (!this.nodes.imageHolder) return;
+
+    this.tunes.forEach((tune) => {
+      this.nodes.imageHolder?.classList.toggle(


seems strange that you check for undefined and then use ?

neSpecc · 2025-02-17T10:36:10Z

src/types.ts

lack of jsdoc

neSpecc · 2025-02-17T10:37:00Z

tsconfig.json

+    "module": "ESNext",
+    "moduleResolution": "node",
+    "strict": true,
+    "jsx": "preserve",


please check carefully every option you're added, it should have particular meaning for this project

neSpecc · 2025-03-14T18:44:50Z

package.json

  "scripts": {
    "dev": "vite",
-    "build": "vite build"
+    "build": "tsc && vite build"


Why do we have both tsc and vite for compiling? For declarations generation we can use vite-plugin-dts like in Image Tool
https://github.com/editor-js/image/blob/master/vite.config.js

neSpecc · 2025-03-14T18:45:22Z

package.json

+    "@types/dompurify": "^3.0.5",
+    "@types/node": "^22.13.4",
+    "@vitejs/plugin-react": "^4.3.4",


why they are used?

neSpecc · 2025-03-14T18:45:39Z

src/index.ts

 * Build styles
 */
-import './index.css';
+import "./index.css";


still actual

neSpecc · 2025-03-14T18:47:10Z

src/index.ts

   */
-  constructor({ data, config, api, readOnly }) {
+  constructor({
+    data = {} as Partial<SimpleImageData>,


actually data can't be an empty object. As well and partial

neSpecc · 2025-03-14T18:47:25Z

src/types.ts

@@ -0,0 +1,20 @@
+import type { API } from '@editorjs/editorjs';
+
+export interface SimpleImageData {


jsdocs missed

…impleImage types documentation

Migrate to TypeScript and update project configuration

38890ee

amriksingh0786 changed the title ~~Migrate to TypeScript and update project configuration~~ Migrate to TypeScript and update project configuration, issue #72 Feb 17, 2025

neSpecc requested changes Feb 17, 2025

View reviewed changes

Refactor TypeScript types and improve code documentation

9504de8

neSpecc reviewed Mar 14, 2025

View reviewed changes

neSpecc requested changes Mar 14, 2025

View reviewed changes

Update build script, add TypeScript declaration plugin, and enhance S…

5ec3c64

…impleImage types documentation

amriksingh0786 requested a review from neSpecc March 19, 2025 11:19

		@@ -0,0 +1,20 @@
		import type { API } from '@editorjs/editorjs';

		export interface SimpleImageData {

Migrate to TypeScript and update project configuration, issue #72 #76

Are you sure you want to change the base?

Migrate to TypeScript and update project configuration, issue #72 #76

Uh oh!

Conversation

amriksingh0786 commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amriksingh0786 commented Feb 17, 2025 •

edited

Loading