Skip to content

migrate miscellaneous files to typescript#12308

Open
k-yle wants to merge 1 commit intodevelopfrom
kh/ts-minor
Open

migrate miscellaneous files to typescript#12308
k-yle wants to merge 1 commit intodevelopfrom
kh/ts-minor

Conversation

@k-yle
Copy link
Copy Markdown
Collaborator

@k-yle k-yle commented May 8, 2026

originally I was just going to edit util/* (since future PRs depend on these files), but I added some other files that are trivial to convert

@k-yle k-yle added the chore Improvements to the iD development experience or codebase label May 8, 2026
keybinding.unbind = function(selection) {
_keybindings = [];
keybinding.unbind = function(selection?: d3.Selection) {
_keybindings = {};
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this was presumably a typo, that happens to still works

return keybinding.off(codes, capture);
}

var arr = utilArrayUniq([].concat(codes));
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this shallow clone is redundant since utilArrayUniq does not mutate its input

Copy link
Copy Markdown
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

Thanks! 🤗

Only very partial review of the PR, but see a few details I noticed already below.

Comment thread modules/util/object.ts
@@ -1,26 +1,24 @@
export function utilObjectOmit(obj, omitKeys) {
return Object.keys(obj).reduce(function(result, key) {
export function utilObjectOmit<T extends object, K extends keyof T>(obj: T, omitKeys: K[]): Omit<T, K> {
Copy link
Copy Markdown
Member

@tyrasd tyrasd May 8, 2026

Choose a reason for hiding this comment

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

this could be replaced by es-toolkit's omit?

//edit: this is probably also true for a lot of the methods in array.ts which also exist identically in es-toolkit. Perhaps it's better to migrate them to typescript as proposed, and replace them with es-toolkit's methods in a later step separately.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah i agree, shall we do this in a separate PR?

Comment thread modules/util/object.ts Outdated
Comment thread modules/util/dimensions.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Improvements to the iD development experience or codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants