search

Esri / wayback

Esri World Imagery Wayback App - Explore, compare and start using all different versions of World Imagery Basemap released since 2014
https://livingatlas.arcgis.com/wayback/
Apache License 2.0
18 stars 3 forks source link

Fixed bugs in reducer and updated TypeScript types #101

Closed thekester closed 2 months ago

thekester commented 2 months ago

Hello! I encountered several errors when running npm start, so I corrected them and now everything is working fine. This Pull Request addresses the issues related to type errors, indexing problems, and Tailwind CSS configuration in the Wayback project. The following changes were made:

1. Fixed TypeScript Type Errors:
    Added explicit type definitions for the byReleaseNumber object to handle number index signatures correctly. This resolves the TS7053 error encountered when trying to index objects using number types.

2. Updated getPreloadedState Function:
    Refined the type definitions for the DownloadJob and ensured compatibility with the existing types. This includes explicitly defining the structure of byId to avoid type mismatches.

3. Enhanced Reducer Type Safety:
    Updated the initial state and reducer logic in reducer.ts to include precise type annotations. This ensures that TypeScript can correctly infer types and prevent runtime errors.

4. Updated Tailwind CSS Configuration:

Modified tailwind.config.js to update the content paths and add custom colors, ensuring the proper application of Tailwind styles.

Changes Made

src/store/Wayback/reducer.ts:
    Added explicit type annotations for the byReleaseNumber object.
    Updated the itemsLoaded reducer to correctly handle number indices.

src/store/getPreloadedState.ts:
    Defined the DownloadJob interface to include all necessary properties.
    Ensured the byId object is typed correctly to prevent type errors.
thekester commented 2 months ago

Errors Encountered and Corrections Made

During the execution of npm start, several TypeScript errors were encountered. Below is a detailed description of each error and the corrections that were made:

Error in createWebmap.ts line 197

Error:

ERROR in ./src/components/SaveAsWebmapDialog/createWebmap.ts:197:30
TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ title: string; description: string; tags: string; extent: string; snippet: string; text: string; type: string; overwrite: string; f: string; token: string; }'.
No index signature with a parameter of type 'string' was found on type '{ title: string; description: string; tags: string; extent: string; snippet: string; text: string; type: string; overwrite: string; f: string; token: string; }'.
195 |
196 |     Object.keys(uploadRequestContent).map((key) => {
197 |         formData.append(key, uploadRequestContent[key]);
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^
198 |     });
199 |
200 |     try {

Solution: Added a type assertion to allow indexing by string.

Object.keys(uploadRequestContent).map((key) => {
    formData.append(key, (uploadRequestContent as { [key: string]: any })[key]);
});

Error in Switch.tsx line 25

Error:

ERROR in ./src/components/SettingDialog/Switch.tsx:25:9
TS7053: Element implicitly has an 'any' type because expression of type '"checked"' can't be used to index type '{}'.
Property 'checked' does not exist on type '{}'.
23 |
24 |     if (checked) {
25 |         props['checked'] = true;
    |         ^^^^^^^^^^^^^^^^
26 |     }
27 |
28 |     useEffect(() => {

Solution: Defined a generic type for props to allow dynamic properties.

const props: { [key: string]: any } = {};
if (checked) {
    props['checked'] = true;
}

Error in getPreloadedState.ts line 189

Error:

ERROR in ./src/store/getPreloadedState.ts:189:9
TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'.
No index signature with a parameter of type 'string' was found on type '{}'.
187 |     for (const job of jobs) {
188 |         const { id } = job;
189 |         byId[id] = job;
    |         ^^^^^^^^
190 |         ids.push(id);
191 |     }
192 |

Solution: Defined the byId object with appropriate type to allow string indexing.

interface Job {
    id: string;
    // other properties
}

const byId: { [key: string]: Job } = {};
const ids: string[] = [];

for (const job of jobs) {
    const { id } = job;
    byId[id] = job;
    ids.push(id);
}

Error in reducer.ts line 78

Error:

ERROR in ./src/store/Wayback/reducer.ts:78:17
TS7053: Element implicitly has an 'any' type because expression of type 'number' can't be used to index type '{}'.
No index signature with a parameter of type 'number' was found on type '{}'.
76 |             for (const item of items) {
77 |                 const { releaseNum } = item;
78 |                 byReleaseNumber[releaseNum] = item;
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
79 |                 allReleaseNumbers.push(releaseNum);
80 |             }
81 |

Solution: Defined the byReleaseNumber object with appropriate type to allow number indexing.

const byReleaseNumber: { [key: number]: IWaybackItem } = {};
const allReleaseNumbers: number[] = [];

for (const item of items) {
    const { releaseNum } = item;
    byReleaseNumber[releaseNum] = item;
    allReleaseNumbers.push(releaseNum);
}

state.byReleaseNumber = byReleaseNumber;
state.allReleaseNumbers = allReleaseNumbers;

Additional Changes

Tailwind CSS Configuration:

Updated the tailwind.config.js file to extend colors and adjust content paths.

Warnings:

Solution:

module.exports = {
  content: [
    './src/**/*.html',
    './src/**/*.{js,jsx,ts,tsx}',
  ],
  darkMode: 'class', // or 'media' or false
  theme: {
    extend: {
      colors: {
        sky: require('tailwindcss/colors').sky,
        stone: require('tailwindcss/colors').stone,
        neutral: require('tailwindcss/colors').neutral,
        gray: require('tailwindcss/colors').gray,
        slate: require('tailwindcss/colors').slate,
        custom: {
          'modal': {
            'background': 'rgba(26,61,96, 0.9)',
            'content-background': '#000'
          },
          'theme': {
            'blue': {
              DEFAULT: '#2267AE',
              'brand': '#0079c1',
              'light': '#56a5d8',
              'dark': '#1A3D60'
            }
          },
          'background': {
            DEFAULT: '#121212'
          },
          'foreground': {
            DEFAULT: '#ccc'
          }
        }
      },
    },
  },
  variants: {
    extend: {},
  },
  plugins: [],
}
vannizhang commented 2 months ago

Hi @thekester,

Thank you for submitting this PR and for your efforts in addressing the errors you encountered.

The fixes you made for the TypeScript errors look good to me, so I will merge this PR into the master branch.

However, I will revert the changes you made to the README and tailwind.config.js files.

Jinnan

thekester commented 2 months ago

Hi @vannizhang,

No problem, thank you for reviewing and merging the PR!

Theophile