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

Remove legacy contextTypes #4997

Merged
merged 9 commits into from
Jul 18, 2024
Merged

Remove legacy contextTypes #4997

merged 9 commits into from
Jul 18, 2024

Conversation

p2edwards
Copy link
Contributor

@p2edwards p2edwards commented Jul 4, 2024

Description

Remove remnants of legacy Context API usage that was causing React 18 to show deprecation warnings in the developer console. Also removes the 'prop-types' library and converts the reactCreateBemElement function to TypeScript.

Notes for reviewer

This PR

  • Removes contextTypes, reducing console.error noise from functions deprecated in React 18
  • TypeScript-izes reactBemComponents, replacing the last use of 'prop-types'
  • Removes the 'prop-types' dependency

More info:

Test this:

  • KoboMatrix, which is the only thing that uses reactBemComponents.tsx right now. (I didn't notice right away that the rest of the app was moved over to bem.ts / makeBem ).

Checklist

  1. If you've added code that should be tested, add tests
  2. If you've changed APIs, update (or create!) the documentation
  3. Ensure the tests pass
  4. Make sure that your code lints and that you've followed our coding style
  5. Write a title and, if necessary, a description of your work suitable for publishing in our release notes
  6. Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE)
  7. Open an issue in the docs if there are UI/UX changes

Related issues

Follow-up to #4992

This removes some legacy api deprecation notices in the console in
React 18.

- Replace the last this.context.router in projectSettings.es6 with
  this.props.router from withRouter HOC. (Tested this, works fine)

- Remove these, which are unused:

      SomeComponent.contextTypes = {
        router: PropTypes.object
      };
@p2edwards p2edwards added dependencies Pull requests that update a dependency file Front end labels Jul 4, 2024
Doing this step in one commit, hoping it will make the diff easier to
review online.
@magicznyleszek magicznyleszek self-assigned this Jul 4, 2024
@magicznyleszek magicznyleszek self-requested a review July 4, 2024 20:10
e.g., not say 'duplicate import' for this code:

   import React from 'react'
   import type { ReactNode } from 'react'

…which used to work fine, but typescript-eslint's no-duplicate-imports
rule has been removed:

- typescript-eslint/typescript-eslint@47eeea9275bd
- https://typescript-eslint.io/rules/no-duplicate-imports/

We could do more with this plugin.

Only skimmed it, but some of the other features might require some
more config.

- https://github.com/import-js/eslint-plugin-import/blob/main/README.md#resolvers

(Meanwhile, I'll try not to think about how many dev dependencies
we bring in just for this cool rule.)
- Remove modifier='[object Object]' in generated bem elements
- Use the new React 18 root API in renderInBackbone
@p2edwards p2edwards changed the title WIP: Remove legacy contextTypes Remove legacy contextTypes Jul 5, 2024
@magicznyleszek magicznyleszek merged commit 70f97e6 into beta Jul 18, 2024
5 checks passed
@magicznyleszek magicznyleszek deleted the remove-context-types branch July 18, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Front end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants