Migrate to TypeScript and update project configuration, issue #72 #76
Conversation
amriksingh0786
commented
Feb 17, 2025
•
amriksingh0786
commented
amriksingh0786
changed the title
| @@ -1,9 +1,16 @@ | |||
| /** | |||
| * Build styles | |||
| */ | |||
| import './index.css'; | |||
| import "./index.css"; | |||
src/index.ts
Outdated
| // 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; | ||
| } |
There was a problem hiding this comment.
these interfaces are redundant since editor.js package already includes these types
src/index.ts
Outdated
| export default class SimpleImage { | ||
| private api: any; |
There was a problem hiding this comment.
specify exact type instead of any
src/index.ts
Outdated
| type: "pattern"; | ||
| data: string; | ||
| } | ||
|
|
||
| export default class SimpleImage { |
There was a problem hiding this comment.
class should implement BlockTool
src/index.ts
Outdated
| @@ -27,7 +75,12 @@ export default class SimpleImage { | |||
| * api - Editor.js API | |||
| * readOnly - read-only mode flag | |||
| */ | |||
| constructor({ data, config, api, readOnly }) { | |||
| constructor({ | |||
| data = {} as Partial<SimpleImageData>, | |||
There was a problem hiding this comment.
it is the logic change, please, explain it
There was a problem hiding this comment.
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
There was a problem hiding this comment.
actually data can't be an empty object. As well and partial
|
|
||
| if (!image) { | ||
| return this.data; | ||
| } | ||
|
|
||
| return Object.assign(this.data, { | ||
| url: image.src, | ||
| caption: caption.innerHTML, | ||
| url: image?.src || "", |
There was a problem hiding this comment.
image and caption cant be undefined, so this logic never be used
| if (!this.nodes.imageHolder) return; | ||
|
|
||
| this.tunes.forEach((tune) => { | ||
| this.nodes.imageHolder?.classList.toggle( |
There was a problem hiding this comment.
seems strange that you check for undefined and then use ?
tsconfig.json
Outdated
| "module": "ESNext", | ||
| "moduleResolution": "node", | ||
| "strict": true, | ||
| "jsx": "preserve", |
There was a problem hiding this comment.
please check carefully every option you're added, it should have particular meaning for this project
package.json
Outdated
| @@ -24,17 +25,24 @@ | |||
| }, | |||
| "scripts": { | |||
| "dev": "vite", | |||
| "build": "vite build" | |||
| "build": "tsc && vite build" | |||
There was a problem hiding this comment.
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
package.json
Outdated
| "@types/dompurify": "^3.0.5", | ||
| "@types/node": "^22.13.4", | ||
| "@vitejs/plugin-react": "^4.3.4", |
| @@ -1,9 +1,16 @@ | |||
| /** | |||
| * Build styles | |||
| */ | |||
| import './index.css'; | |||
| import "./index.css"; | |||
src/index.ts
Outdated
| @@ -27,7 +75,12 @@ export default class SimpleImage { | |||
| * api - Editor.js API | |||
| * readOnly - read-only mode flag | |||
| */ | |||
| constructor({ data, config, api, readOnly }) { | |||
| constructor({ | |||
| data = {} as Partial<SimpleImageData>, | |||
There was a problem hiding this comment.
actually data can't be an empty object. As well and partial
| @@ -0,0 +1,20 @@ | |||
| import type { API } from '@editorjs/editorjs'; | |||
|
|
|||
| export interface SimpleImageData { | |||
…impleImage types documentation
