Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Prettier changed the formatting here.
| "test": "node --test --import ./test/mock-browser.js", | ||
| "coverage": "node --test --import ./test/mock-browser.js --experimental-test-coverage", | ||
| "test": "tsx test/*.test.ts", | ||
| "coverage": "tsx test/*.test.ts --experimental-test-coverage", |
There was a problem hiding this comment.
For --experimental-test-coverage to run, Node v22 must be used.
| zoomend: function() { | ||
| ctx.store.changeZoom(); | ||
| }, |
There was a problem hiding this comment.
This looked like a dead/old method as store so I removed it.
| "docs": "run-s docs-modes-life-cycle docs-modes-get-and-set", | ||
| "docs-modes-get-and-set": "documentation readme --readme-file ./docs/MODES.md -s \"Setters and Getters\" src/modes/mode_interface_accessors.js --shallow", | ||
| "docs-modes-life-cycle": "documentation readme --readme-file ./docs/MODES.md -s \"Life Cycle Functions\" src/modes/mode_interface.js --shallow", | ||
| "lint": "eslint index.js src test bench", | ||
| "docs-modes-get-and-set": "documentation readme --readme-file ./docs/MODES.md -s \"Setters and Getters\" src/modes/mode_interface_accessors.ts --shallow", | ||
| "docs-modes-life-cycle": "documentation readme --readme-file ./docs/MODES.md -s \"Life Cycle Functions\" src/modes/mode_interface.ts --shallow", |
There was a problem hiding this comment.
Not sure what to do here. JSdoc have largely been replaced with TypeScript 🤔
|
|
||
| assert.deepEqual( | ||
| Draw.getAll().features[0].geometry.coordinates, | ||
| [[undefined]], |
There was a problem hiding this comment.
This needs to be checked. I'm not quite sure why it changed from null to undefined.
|
This is awesome. Would love too see this get merged. :) |
There was a problem hiding this comment.
It's amazing job! Thank you! 🥳
I made the first iteration of review and left some comments. Could you please also ensure that some changes marked in Git as renaming and not delete/add because it's hard to see the difference, especially for large files (see screenshot)
and also rename in Git will retain Git history
And also I'd prefer to apply prettier as a separate PR, but it's up to you.
|
|
||
| const ctx = { | ||
| options | ||
| } as spy; |
There was a problem hiding this comment.
Why do we have this test-related cast here?
| name: 'MapboxDraw', | ||
| file: outputFile, | ||
| format: 'umd', | ||
| format: 'esm', |
There was a problem hiding this comment.
It seems like a breaking change for customers who use this library through a script tag.
| @@ -0,0 +1 @@ | |||
| declare module 'fast-deep-equal'; | |||
There was a problem hiding this comment.
But fast-deep-equal has typings out of the box based on NPM
| @@ -0,0 +1 @@ | |||
| declare module '@mapbox/geojson-area'; | |||
| @@ -0,0 +1 @@ | |||
| declare module '@mapbox/point-geometry'; | |||
| @@ -23,7 +23,6 @@ | |||
| ], | |||
| "type": "module", | |||
| "exports": { | |||
There was a problem hiding this comment.
I guess now we can expose types for GL Draw as we do in GL JS https://github.com/mapbox/mapbox-gl-js/blob/main/package.json#L130
| @@ -1,4 +1,5 @@ | |||
| import ModeInterface from './mode_interface.js'; | |||
| import type { CTX, DrawCustomMode, StrictFeature } from '../types/types'; | |||
|
Interested on this been merged. |
5 participants
This PR replaces
.jsfiles insrc/andtest/with.ts. Additionally:npm run tscto evaluate typingsNote: I haven't touched benchmarks. Things remain JavaScript there.
dist/mapbox-gl-draw-unminified.jsbundles at113kdist/mapbox-gl-draw.jsbundles at51k