Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Create slices #21

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Create slices #21

wants to merge 9 commits into from

Conversation

johnmarkredding
Copy link
Contributor

@johnmarkredding johnmarkredding commented Jun 15, 2024

This is too big of a pull, I'm sure. Let me know how you'd prefer to do this.
I've tried to test each of the affected files as I've made changes. There are a couple things in the math files and selectors that I haven't figured out, so I haven't updated those fully yet.

Summary by Sourcery

This pull request introduces significant refactoring and enhancements to the state management of the application by implementing Redux slices and TypeScript types. It also updates the ESLint and Rollup configurations for improved code quality and build process. Additionally, it removes deprecated Redux actions, reducers, and selectors.

  • New Features:
    • Introduced Redux slices for plugin, model, metric, skewt, and wind state management.
    • Added TypeScript types for Redux actions and state to improve type safety.
    • Implemented new hooks for using typed dispatch and selector in the application.
  • Enhancements:
    • Refactored the Redux store configuration to use slices and combined reducers.
    • Updated ESLint configuration to include new rules and plugins for better code quality.
    • Refactored various components to use the new Redux slices and TypeScript types.
    • Improved the Rollup configuration for better module resolution and build process.
  • Build:
    • Updated Rollup configuration to use consistent import styles and added new plugins for better build optimization.
  • Chores:
    • Removed old Redux actions, reducers, and selectors that have been replaced by the new slices.

Conform tsconfig.json to formatting rules

Update config

Allow for subfolder typescript linting

Conform to eslint rules

Remove redundant fallthrough rule

Include tsRules for tsx files

Fix react in jsx scope linter warnings

Remove unecessary propTypes check due to use of TS
Ensure import order, and fix import filetype

Conform to eslint rules

Fix import order

Conform to eslint rules

Fix indentation

Fix formatting
Create Model Slice

Use EnhancedStore

Use root level imports, not pwd paths.

Clarify naming overlap between store definition and params

Add types for modelSlice reducers

Export modelSlice actions

Export accurate RootState and AppDispatch types

Unify model reducer action types

Create windSlice.ts

Create skewtSlice

Rename skewt pMin action type

Create SkewtState

Create metricSlice.ts

Consistently type metricSlice initialState

Update metric types

Specify modelSlice types

Allow for list of params, convert to payload object

Move reducers to slices

Create pluginSlice

Update setModelName import path for new slice structure

Move setModelName to modelSlice

Create cancelSubscription thunk

Update updateMetrics import path

Move updateMetrics from store.ts to metricSlice

Revert modelName filtering correctly

Remove outdated redux actions and reducers

Convert LoadingIndicator to functional component

Import actions from new pluginSlice

Fix store import to include typing

Include pressure in MetricState

Import actions from pluginSlice, not old actions file

Access store by import rather than getter function

Explicitly type debugClouds to avoid poor inference

modify state selectors to match slice state naming

Reconfigure store definition, for cleaner importing into components

Rename import locations using root relative paths

Futureproof flexibility of typing receiveParams reducer

Prefer parameters object over args

Add typing to selectors to prevent malformed accessing of state

Match updateMetrics to thunk pattern

Remove unnecessary anonymous functions

Dispatch model name updates

Fix improper typing of modelName due to 'as const' on model prefixes

rename getStore to updateStore to clarify purpose

Organize and modify Selectors

Simplify imports

Create index.ts for all actions and reducers

Move selectors to features folder

Import selectors from new location, and rename soundingSel to pluginSel

Remove old selectors

Add typing to math.ts

Add types to clouds.ts

Fix selectAt return type to number

Copy arrays for sorting, due to mutation of .sort()

Convert to functional component and add types

use keys to make JSX valid

Prevent error on selection of dropdown default value after selecting a valid fav

Add event types and use .currentTarget in place of .target, to accurately access value of the SelectElement

Retain class component

Include default render function, and props and state typing for PureComponent

Return LoadingIndicator to class component, with proper props desctructuring

Restrict MetricState types further

set number as yPointer type

Add flexible types for PureComponent

Add types and prevent naming clash

Add key for react array optimization and compliance

Use function instead of const to prevent 'used before declared' errors

Add types to wind.tsx

use regular class component destructuring to comply with render method signature
Copy link

sourcery-ai bot commented Jun 15, 2024

Reviewer's Guide by Sourcery

This pull request introduces significant refactoring and enhancements to the codebase, primarily focusing on the creation of slices for Redux state management, TypeScript type improvements, and various configuration updates. The changes include the addition of new Redux slices, updates to ESLint and Rollup configurations, and the refactoring of several components and utility functions to improve type safety and maintainability.

File-Level Changes

Files Changes
src/features/plugin/pluginSlice.ts
src/features/model/modelSlice.ts
src/features/metric/metricSlice.ts
src/features/index.ts
src/features/wind/windSelector.ts
src/features/skewt/skewtSlice.ts
src/features/wind/windSlice.ts
Created new Redux slices for plugin, model, metric, skewT, and wind state management. Combined all slices into a root reducer and exported actions and selectors.
src/util/store.ts
src/containers/containers.tsx
src/components/wind.tsx
src/sounding.tsx
src/util/math.ts
src/components/favorites.tsx
src/selectors/sounding.ts
src/selectors/skewt.ts
src/util/clouds.ts
src/components/pure.ts
src/components/loading.tsx
src/components/skewt.tsx
src/components/parcel.tsx
Refactored components, selectors, and utility functions to use new Redux slices and TypeScript types. Updated import paths and removed deprecated functions.
.eslintrc.cjs
rollup.config.js
tsconfig.json
Updated ESLint, Rollup, and TypeScript configurations for better readability, maintainability, and type safety.
src/reducers/sounding.ts
src/actions/sounding.ts
src/selectors/wind.ts
src/reducers/skewt.ts
src/actions/skewt.ts
src/reducers/wind.ts
Deleted old reducers, actions, and selectors for sounding, skewT, and wind state management.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @johnmarkredding - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 7 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

'no-import-assign': 'warn',
'a11y-no-static-element-interactions': 'off',
'a11y-missing-attribute': 'off',
"react/react-in-jsx-scope": "off",
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider adding a comment explaining why this rule is turned off.

Disabling the 'react/react-in-jsx-scope' rule can be confusing for future developers. Adding a comment explaining the rationale behind this decision would be helpful.

Suggested change
"react/react-in-jsx-scope": "off",
// Disabling this rule because React 17+ with the new JSX transform
// no longer requires React to be in scope.
"react/react-in-jsx-scope": "off",

if (store) {
return store;
import { configureStore, ThunkAction, Action } from '@reduxjs/toolkit';
import rootReducer, {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider renaming the function for clarity.

The function 'updateStore' might be better named as 'initializeStore' or 'configureStore' to better reflect its purpose, which is to set up the store initially.

Suggested change
import rootReducer, {
import { configureStore, ThunkAction, Action } from '@reduxjs/toolkit';
import rootReducer, {
setWidth,
setHeight,
} from './rootReducer';

import { sampleAt } from "../util/math";

export class WindGram extends PureComponent {
constructor(props) {
type WindGramProps = {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider adding JSDoc comments for the props.

Adding JSDoc comments for the 'WindGramProps' type can improve code readability and provide better context for future developers.

Suggested change
type WindGramProps = {
/**
* Props for the WindGram component.
* @property {boolean} [isLoading] - Indicates if the component is in a loading state.
* @property {ReturnType<typeof skewTSel.params>} [params] - Parameters for the skewTSel function.
*/
type WindGramProps = {
isLoading?: boolean;
params?: ReturnType<typeof skewTSel.params>;
};

import windyStore from "@windy/store";
import windyUtils from "@windy/utils";
import windyModels from "@windy/models";

function label(favorite) {
type FavoritesProps = {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider adding JSDoc comments for the props.

Adding JSDoc comments for the 'FavoritesProps' type can improve code readability and provide better context for future developers.

Suggested change
type FavoritesProps = {
/**
* Props for the Favorites component.
* @property {PluginState["favorites"]} favorites - List of favorite items.
* @property {string} location - Current location.
*/
type FavoritesProps = {
favorites: PluginState["favorites"];
location: string;
};

@@ -0,0 +1,177 @@
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider splitting the slice into smaller slices.

The 'pluginSlice' is handling a lot of state and actions. Consider splitting it into smaller, more focused slices to improve maintainability and readability.

Suggested change
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import pluginStateSlice from "./pluginStateSlice";
import pluginActionsSlice from "./pluginActionsSlice";

@@ -0,0 +1,158 @@
import windyUtils from "@windy/utils";
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider adding a comment explaining the purpose of the slice.

Adding a comment at the top of the file explaining the purpose of the 'modelSlice' can provide better context for future developers.

Suggested change
import windyUtils from "@windy/utils";
/**
* This file defines the 'modelSlice' which is responsible for managing the state
* related to the model feature in the application. It utilizes utilities, subscriptions,
* and products from the Windy library to handle various functionalities.
*/
import windyUtils from "@windy/utils";

@@ -0,0 +1,6 @@
import { useDispatch, useSelector } from 'react-redux';
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider adding a comment explaining the custom hooks.

Adding a comment explaining the purpose and usage of the custom hooks 'useAppDispatch' and 'useAppSelector' can improve code readability and provide better context for future developers.

Suggested change
import { useDispatch, useSelector } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';
// Custom hooks to use throughout the app for dispatching actions and selecting state
import type { AppDispatch, RootState } from 'src/util/store';

@@ -13,11 +14,11 @@ for (let i = 0; i < 160; i++) {
// Output an object:
// - clouds: the clouds cover,
// - width & height: dimension of the cover data.
export function computeClouds(airData) {
export function computeClouds(airData: MeteogramDataPayload) {
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider simplifying the code while maintaining type safety.

The new code introduces additional complexity without significant benefits. Here are some points to consider:

  1. Type Annotations: While type annotations can improve code safety, they should be added thoughtfully. In this case, some annotations are redundant (e.g., const numX = airData["rh-1000h"].length as number;).

  2. Unnecessary Generics: The use of generics in cubicInterpolate<T extends number> is unnecessary since all parameters are numbers, adding complexity without benefit.

  3. Verbose Code: The new code is more verbose, making it harder to read and maintain. Repeated type annotations for arrays (number[]) clutter the code.

  4. New Type Definitions: The introduction of the CloudCanvas type adds more lines and complexity. While type definitions can be useful, this seems overkill for a simple function parameter.

Consider simplifying the code while maintaining type safety. Here's a less complex version:

import { lerp } from "./math";

export const hrAlt = [0, 5, 11, 16.7, 25, 33.4, 50, 58.4, 66.7, 75, 83.3, 92, 98, 100];
const hrAltPressure = [null, 950, 925, 900, 850, 800, 700, 600, 500, 400, 300, 200, 150, null];

const lookup = new Uint8Array(256);
for (let i = 0; i < 160; i++) {
  lookup[i] = clampIndex(24 * Math.floor((i + 12) / 16), 160);
}

export function computeClouds(airData: { [key: string]: number[] }) {
  const numX = airData["rh-1000h"].length;
  const numY = hrAltPressure.length;
  const rawClouds = new Array(numX * numY);

  for (let y = 0, index = 0; y < numY; ++y) {
    if (hrAltPressure[y] == null) {
      for (let x = 0; x < numX; ++x) {
        rawClouds[index++] = 0.0;
      }
    } else {
      const weight = hrAlt[y] * 0.01;
      const pAdd = lerp(-60, -70, weight);
      const pMul = lerp(0.025, 0.038, weight);
      const pPow = lerp(1.0, 1.5, weight);
      const pMul2 = lerp(0.5, 0.8, weight);

      for (let x = 0; x < numX; ++x) {
        let f = airData["rh-1000h"][x] + pAdd;
        f = Math.pow(f, pPow) * pMul2;
        rawClouds[index++] = f;
      }
    }
  }

  const sliceWidth = 10;
  const width = sliceWidth * numX;
  const height = 300;
  const clouds = new Array(width * height);
  const kh = (height - 1) * 0.01;
  const dx2 = (sliceWidth + 1) >> 1;
  let heightLookupIndex = 2 * height;
  const heightLookup = new Array(heightLookupIndex);
  const buffer = new Array(16);
  let previousY;
  let currentY = height;

  for (let j = 0; j < numY - 1; ++j) {
    previousY = currentY;
    currentY = Math.round(height - 1 - hrAlt[j + 1] * kh);
    const j0 = numX * clampIndex(j + 2, numY);
    const j1 = numX * clampIndex(j + 1, numY);
    const j2 = numX * clampIndex(j + 0, numY);
    const j3 = numX * clampIndex(j - 1, numY);
    let previousX = 0;
    let currentX = sliceWidth;

    for (let i = 0; i < numX; ++i) {
      const x0 = previousX;
      const x1 = currentX;
      const x2 = currentX + dx2;
      const x3 = currentX + sliceWidth;

      for (let y = previousY; y < currentY; ++y) {
        const y0 = heightLookup[heightLookupIndex - y];
        const y1 = heightLookup[heightLookupIndex - y - 1];
        const y2 = heightLookup[heightLookupIndex - y - 2];
        const y3 = heightLookup[heightLookupIndex - y - 3];

        const m = buffer;
        m[0] = rawClouds[j0 + x0];
        m[1] = rawClouds[j0 + x1];
        m[2] = rawClouds[j0 + x2];
        m[3] = rawClouds[j0 + x3];
        m[4] = rawClouds[j1 + x0];
        m[5] = rawClouds[j1 + x1];
        m[6] = rawClouds[j1 + x2];
        m[7] = rawClouds[j1 + x3];
        m[8] = rawClouds[j2 + x0];
        m[9] = rawClouds[j2 + x1];
        m[10] = rawClouds[j2 + x2];
        m[11] = rawClouds[j2 + x3];
        m[12] = rawClouds[j3 + x0];
        m[13] = rawClouds[j3 + x1];
        m[14] = rawClouds[j3 + x2];
        m[15] = rawClouds[j3 + x3];

        clouds[y * width + x0] = bicubicFiltering(m, 0.5, 0.5);
      }
    }
  }

  return { clouds, width, height };
}

function clampIndex(index: number, size: number) {
  return index < 0 ? 0 : index > size - 1 ? size - 1 : index;
}

function step(x: number) {
  return lookup[Math.floor(clampIndex(x, 160))];
}

function cubicInterpolate(y0: number, y1: number, y2: number, y3: number, m: number) {
  const a0 = -y0 * 0.5 + 3.0 * y1 * 0.5 - 3.0 * y2 * 0.5 + y3 * 0.5;
  const a1 = y0 - 5.0 * y1 * 0.5 + 2.0 * y2 - y3 * 0.5;
  const a2 = -y0 * 0.5 + y2 * 0.5;
  return a0 * m ** 3 + a1 * m ** 2 + a2 * m + y1;
}

function bicubicFiltering(m: number[], s: number, t: number) {
  return cubicInterpolate(
    cubicInterpolate(m[0], m[1], m[2], m[3], s),
    cubicInterpolate(m[4], m[5], m[6], m[7], s),
    cubicInterpolate(m[8], m[9], m[10], m[11], s),
    cubicInterpolate(m[12], m[13], m[14], m[15], s),
    t
  );
}

export function cloudsToCanvas(clouds: number[], width: number, height: number, canvas: HTMLCanvasElement | null) {
  if (canvas == null) {
    canvas = document.createElement("canvas");
  }
  canvas.width = width;
  canvas.height = height;
  const ctx = canvas.getContext("2d");
  const imageData = ctx.getImageData(0, 0, width, height);
  const imgData = imageData.data;

  let srcOffset = 0;
  for (let y = 0; y < height; ++y) {
    for (let x = 0; x < width; ++x) {
      const value = clouds[srcOffset++];
      const color = Math.round(value * 255);
      const offset = (y * width + x) * 4;
      imgData[offset] = color;
      imgData[offset + 1] = color;
      imgData[offset + 2] = color;
      imgData[offset + 3] = 255;
    }
  }

  ctx.putImageData(imageData, 0, 0);
}

This version maintains type safety while being more readable and maintainable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant