Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start using TypeScript #1450

Merged
merged 30 commits into from
May 9, 2023
Merged

Start using TypeScript #1450

merged 30 commits into from
May 9, 2023

Commits on May 5, 2023

  1. Initialize typescript support

    Start a TS config file, add npm scripts, and add CI.
    
    Typescript config is mostly via `tsc -init` and will be modified by
    later commits.
    
    Co-authored-by: James Hadfield <hadfield.james@gmail.com>
    victorlin and jameshadfield committed May 5, 2023
    Configuration menu
    Copy the full SHA
    cb6bd9f View commit details
    Browse the repository at this point in the history
  2. eslint: Configure support for TypeScript

    Follow instructions for typescript-eslint¹, except the addition of
    plugin:@typescript-eslint/recommended - that will come in a future
    commit.
    
    Instead, use plugin:@typescript-eslint/eslint-recommended to have
    compatibility with the existing use of eslint:recommended².
    
    Adjust existing rules to use @typescript-eslint counterparts.
    
    ¹ https://typescript-eslint.io/getting-started
    ² https://typescript-eslint.io/linting/configs#eslint-recommended
    victorlin committed May 5, 2023
    Configuration menu
    Copy the full SHA
    aeb77bc View commit details
    Browse the repository at this point in the history
  3. Fix (new) linting errors

    These weren't flagged as errors previously, however the previous commit
    swapped the rule "no-use-before-define" to "@typescript/no-use-before-define".
    
    These seem to be false-positives (the code works just fine in practice)
    related to named exports. For instance, the following "works"
    but fails linting:
    ```
    export const a = () => b; // error: 'b' was used before it was defined.
    export const b = 42;
    ```
    This is an error even if we enable the "allowNamedExports" option - see
    https://eslint.org/docs/latest/rules/no-use-before-define#allownamedexports
    
    I don't know enough about specifications to know why this is considered
    an error but still works in practice, however it's not much effort to
    reorganise the code in this file.
    jameshadfield authored and victorlin committed May 5, 2023
    Configuration menu
    Copy the full SHA
    af4c742 View commit details
    Browse the repository at this point in the history

Commits on May 8, 2023

  1. eslint: Extend @typescript-eslint/recommended

    This base config, strongly recommended by typescript-eslint¹, contains
    code quality rules for TypeScript code.
    
    Place it after eslint:recommended since this ruleset disables preceding
    rules known to cause conflict. The previous base config is automatically
    included with the new one² so it is a replacement rather than addition.
    
    This exposes new linting errors - add rule overrides to document them
    here and address separately in subsequent commits.
    
    ¹ https://typescript-eslint.io/linting/configs/#recommended
    ² https://typescript-eslint.io/linting/configs/#eslint-recommended
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    941dce7 View commit details
    Browse the repository at this point in the history
  2. eslint: Keep no-var-requires disabled

    There is existing usage of assigning require to a variable, I assume for
    good reason.
    
    Rule doc page: https://typescript-eslint.io/rules/no-var-requires/
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    9bfc2e0 View commit details
    Browse the repository at this point in the history
  3. eslint: Address TS/no-empty-function violations

    The base no-empty-function rule was not enabled by eslint:recommended,
    however TS-eslint's recommended ruleset enabled it which surfaced
    violations.
    
    I opted to allow empty arrow functions since those are used already in a
    few places. That leaves just one non-arrow function which I disabled as
    an exception given presence of surrounding context for its existence.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    da92cc1 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    11be9e4 View commit details
    Browse the repository at this point in the history
  5. webpack: Include TypeScript files

    For resolve.extensions and babel-loader.
    
    For resolve.extensions specifically,
    
    1. '...' is used to append to the default list¹.
    
    2. The order of ts before tsx is somewhat intentional. Even though it
       doesn't make a difference here (in our currently limited use of
       TypeScript), others have reported that the order can matter for
       unclear reasons².
    
    ¹ https://webpack.js.org/configuration/resolve/#resolve-extensions
    ² https://stackoverflow.com/a/49708475/4410590
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    b9ec806 View commit details
    Browse the repository at this point in the history
  6. tsconfig: Set the context for TypeScript usage

    This will frame decisions made in subsequent commits.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    bf6a4d1 View commit details
    Browse the repository at this point in the history
  7. tsconfig: Only include files under src/

    Although there are other JavaScript files outside of src/, the initial
    aim for TypeScript adoption in this project is scoped to the Auspice
    package, whose project files fall under src/.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    a4a4846 View commit details
    Browse the repository at this point in the history
  8. tsconfig: Allow JS files

    This allows TS files to import from JS files.
    
    Don't set checkJs because JS files don't need to be type-checked.
    
    Don't set maxNodeModuleJsDepth because there is no need to include JS
    files from node_modules.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    9b9d762 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    f289e30 View commit details
    Browse the repository at this point in the history
  10. tsconfig: Remove options related to Language and Environment

    Default values suffice. Reasons:
    
    - lib: We don't need anything outside of the default set of type definitions.
    - experimentalDecorators: We aren't using experimental decorators.
    - noLib: We aren't providing type definitions for primitives.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    c4e7f7c View commit details
    Browse the repository at this point in the history
  11. tsconfig: Use "types" to ignore files in node_modules/@types

    node_modules contains declaration files from dependencies which cause
    errors when run through tsc. These errors are not relevant to compiling
    Auspice code.
    
    Setting skipLibCheck=true suppresses checking of *all* declaration
    files, which is a common workaround¹ to remove the errors described
    above. However, this includes custom declaration files in the codebase.
    Even though there aren't any now, they might come later during
    further TypeScript adoption.
    
    An unset `types` setting is the reason why node_modules/@types is
    included for compilation by default². Set it to an empty array since
    there are no declaration files that need to be explicitly included here
    yet. Also remove `typeRoots` since it is targeting the same thing as
    `types`.
    
    ¹ https://stackoverflow.com/a/57653497/4410590
    ² https://www.typescriptlang.org/tsconfig#types
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    80835fc View commit details
    Browse the repository at this point in the history
  12. tsconfig: Don't check TypeScript-managed .d.ts files

    There's no good reason to check declaration files managed by TypeScript.
    
    This also saves a fraction of a second during compilation.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    b5f2d6f View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    4582287 View commit details
    Browse the repository at this point in the history
  14. tsconfig: Error on unreachable code

    This passes now, meaning there isn't any unreachable code detected.
    Enable this check so any unreachable code must be intentional.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    e70c96f View commit details
    Browse the repository at this point in the history
  15. tsconfig: Error on unused labels

    This passes now, meaning there aren't any unused labels detected.
    Enable this check so any unused labels must be intentional.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    85855e4 View commit details
    Browse the repository at this point in the history
  16. tsconfig: Enable commented type checking options

    These all pass now. Enable them so any inconsistencies must be
    intentional.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    eb3ab39 View commit details
    Browse the repository at this point in the history
  17. tsconfig: Don't customize preserveSymlinks

    There are no symlinks in this project, so this is not necessary.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    3eda8d8 View commit details
    Browse the repository at this point in the history
  18. tsconfig: Don't customize resolveJsonModule

    There are no module imports with a ".json" extension in this project, so
    this is not necessary.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    44aed72 View commit details
    Browse the repository at this point in the history
  19. tsconfig: Don't customize noResolve

    The default behavior seems fine - with the way include and allowJS are
    set, this shouldn't have an affect anyways.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    d4e0a8c View commit details
    Browse the repository at this point in the history
  20. tsconfig: Don't customize allowUmdGlobalAccess

    There are no expectations for global UMD exports in this project, so
    this is not necessary.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    ed6ebc2 View commit details
    Browse the repository at this point in the history
  21. tsconfig: Don't customize baseUrl/paths

    There are no module references in this project that require a custom
    base directory or path, so these are not necessary.
    
    In the future, this *might* be applicable for the @extensions webpack
    alias if files emitted by tsc are used instead of babel.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    07619e2 View commit details
    Browse the repository at this point in the history
  22. Configuration menu
    Copy the full SHA
    1671275 View commit details
    Browse the repository at this point in the history
  23. tsconfig: Set moduleResolution

    'node' is required for node_modules to be considered during an import¹.
    
    ¹ https://www.typescriptlang.org/docs/handbook/module-resolution.html#how-nodejs-resolves-modules
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    b4fb762 View commit details
    Browse the repository at this point in the history
  24. JS→TS: src/util/tipRadiusHelpers.ts

    More as a proof-of-principle, but even using a lot of `any`-types is an
    improvement over the previous code.
    jameshadfield authored and victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    11bcb35 View commit details
    Browse the repository at this point in the history
  25. JS→TS: src/util/extensions

    @types/node must be installed and "node" must be added to types so that
    "process" can be understood.
    
    "noPropertyAccessFromIndexSignature" must be disabled so that
    `process.env.EXTENSION_DATA` can be used without error.
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    ada2db9 View commit details
    Browse the repository at this point in the history
  26. JS→TS: src/components/controls/controls.tsx

    This requires bumping react-i18next to v11.15.6, the earliest version to
    contain a bug fix for TypeScript ≥ 4.6¹.
    
    ¹ i18next/react-i18next@306e03a
    victorlin committed May 8, 2023
    Configuration menu
    Copy the full SHA
    b888bad View commit details
    Browse the repository at this point in the history

Commits on May 9, 2023

  1. Install @types/leaflet

    This is a band-aid fix to an issue with tsc on src/map.js using
    leaflet-gesture-handling's TS declaration file, which requires leaflet
    types that are not provided by the leaflet package.
    
    See this GitHub PR discussion¹ for a complete explanation and
    alternative solutions.
    
    ¹ #1450 (comment)
    victorlin committed May 9, 2023
    Configuration menu
    Copy the full SHA
    7843410 View commit details
    Browse the repository at this point in the history