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

Offer to upgrade tests to support a higher version of react-redux #10127

Closed
1 task done
fkellner opened this issue Mar 26, 2024 · 0 comments · Fixed by #10142
Closed
1 task done

Offer to upgrade tests to support a higher version of react-redux #10127

fkellner opened this issue Mar 26, 2024 · 0 comments · Fixed by #10142

Comments

@fkellner
Copy link
Contributor

Description

In our version of MapStore, we are currently using a newer version of react-redux, i.e. 7.1.3 instead of 6.0.0, because it has a much cleaner syntax.

However, this breaks tests, because of the render-function from react-dom, which is frequently being used like this:

// StandardAppComponent-test
const app = ReactDOM.render(<Provider store={store}><StandardAppComponent/></Provider>, document.getElementById("container"));
expect(app).toExist();

render does not return anything for stateless components:

Render a React element into the DOM in the supplied container and return a reference to the component (or returns null for stateless components).

And apparently, by upgrading to react-redux 7.x, some components become stateless.

A first step to enable you to upgrade to react-redux 7.x and enable us to keep using it would be to rewrite such tests like this:

const container = document.getElementById("container");
expect(container.innerHTML).toNotExist();
ReactDOM.render(<Provider store={store}><StandardAppComponent/></Provider>, container, () => {
    expect(container.innerHTML).toExist();
    done();
});

Would you accept a pull request with these changes to the tests?

This does not break compatibility with your current react-redux version and might also help with upgrading the tests once you reach
React 18 (where render is deprecated entirely).

What kind of improvement you want to add? (check one with "x", remove the others)

  • Refactoring (no functional changes, no api changes)

Other useful information

This concerns the tests for

  • StandardContainer
  • StandardAppComponent
  • StandardRouter
  • ZoomButton
  • ZoomToMaxExtentButton
  • HomeComponent
  • PluginsContainer
  • PaginationToolbar
  • MapViewerCmp
  • PluginsUtils
@fkellner fkellner changed the title Upgrade Tests to support a higher version of react-redux Offer to upgrade tests to support a higher version of react-redux Mar 28, 2024
fkellner pushed a commit to fkellner/MapStore2 that referenced this issue Apr 2, 2024
…ething to enable react-redux 7.x upwards

On Behalf of DB Systel
fkellner pushed a commit to fkellner/MapStore2 that referenced this issue Apr 2, 2024
Updates tests relying on 'render' returning a reference to enable
react-redux 7.x upwards (where some components become stateless and render
no longer returns a reference, even though it was successful)

On Behalf of DB Systel
@tdipisa tdipisa linked a pull request Apr 4, 2024 that will close this issue
2 tasks
@tdipisa tdipisa added this to the 2024.01.01 milestone Apr 4, 2024
@tdipisa tdipisa added the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Apr 4, 2024
@tdipisa tdipisa modified the milestones: 2024.01.01, 2024.02.00 May 14, 2024
@MV88 MV88 closed this as completed in 4985551 Aug 29, 2024
@tdipisa tdipisa removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Aug 30, 2024
@ElenaGallo ElenaGallo self-assigned this Sep 11, 2024
offtherailz pushed a commit that referenced this issue Oct 18, 2024
Updates tests relying on 'render' returning a reference to enable
react-redux 7.x upwards (where some components become stateless and render
no longer returns a reference, even though it was successful)

On Behalf of DB Systel

Co-authored-by: Florian Kellner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants