PB-1383: Migrate Script #1435
PB-1383: Migrate Script #1435
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates JavaScript files to TypeScript to improve type safety across the codebase. The changes convert a pnpm workspace update script and an external layers provider checking script from JavaScript to TypeScript, along with adding proper type annotations and updating import statements.
- Migrated
update-pnpm-workspace.jsto TypeScript with comprehensive interface definitions - Converted
check-external-layers-providersscript to TypeScript with proper type annotations - Updated package dependencies and script references to support TypeScript execution
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/update-pnpm-workspace.ts | Converted from JS to TS with interface definitions and type annotations |
| packages/viewer/src/modules/menu/components/advancedTools/ImportCatalogue/utils.js | Added new utility functions for WMS/WMTS URL detection and processing |
| packages/viewer/scripts/check-external-layers-providers.ts | Migrated from JS to TS with type annotations and updated API imports |
| packages/viewer/package.json | Updated script reference to use .ts extension |
| package.json | Updated workspace script to use tsx and added TypeScript dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /** | ||
| * Checks if the URL is a WMTS url | ||
| * | ||
| * @param {string} urlreturn SetWmsUrlParameters(new URL(provider), language) |
There was a problem hiding this comment.
The JSDoc comment contains malformed text. It appears to include code (urlreturn SetWmsUrlParameters(new URL(provider), language)) that should be just the parameter description.
| * @param {string} urlreturn SetWmsUrlParameters(new URL(provider), language) | |
| * @param {string} url URL to check |
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| import yargs from 'yargs' | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
Using @ts-ignore comments to suppress TypeScript errors for yargs imports suggests missing type definitions. Consider installing @types/yargs or using proper TypeScript-compatible alternatives instead of suppressing type checking.
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | |
| // @ts-ignore | |
| import yargs from 'yargs' | |
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | |
| // @ts-ignore | |
| import yargs from 'yargs' |
| .alias('h', 'help').argv | ||
|
|
||
| function setupAxiosRetry() { | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ |
There was a problem hiding this comment.
Disabling the no-explicit-any rule globally for the entire file reduces type safety benefits of TypeScript. Consider defining proper interfaces for the objects being used instead of relying on 'any' types.
| redirectHeaders, | ||
| checkProviderResponseContentGetMap | ||
| ) | ||
| if (getTileUrl) { |
There was a problem hiding this comment.
[nitpick] The conditional check for getTileUrl wraps the entire fetch and response handling logic, but the corresponding closing brace is at line 206. This creates deeply nested code that could be refactored for better readability by using early returns or separate functions.
ismailsunni
force-pushed
the
feat-pb-1383-migrate-script
branch
from
7831fee to
32fcb27
Compare
- Migrated the translation script - Also started reworking the external provider script by scrapping as many 'any' as possible.
2 participants
Test link