Migrate to TypeScript and update project configuration, issue #72#76
Migrate to TypeScript and update project configuration, issue #72#76amriksingh0786 wants to merge 3 commits intoeditor-js:masterfrom
Conversation
amriksingh0786
changed the title
| * Build styles | ||
| */ | ||
| import './index.css'; | ||
| import "./index.css"; |
| // 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
| } | ||
|
|
||
| export default class SimpleImage { | ||
| private api: any; |
There was a problem hiding this comment.
specify exact type instead of any
| data: string; | ||
| } | ||
|
|
||
| export default class SimpleImage { |
There was a problem hiding this comment.
class should implement BlockTool
| */ | ||
| 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
| 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 ?
| "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
| "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
| "@types/dompurify": "^3.0.5", | ||
| "@types/node": "^22.13.4", | ||
| "@vitejs/plugin-react": "^4.3.4", |
| * Build styles | ||
| */ | ||
| import './index.css'; | ||
| import "./index.css"; |
| */ | ||
| 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
2 participants
Uh oh!
There was an error while loading. Please reload this page.