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

Open
1 task done
fkellner opened this issue Mar 26, 2024 · 0 comments · May be fixed by #10142
Open
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 · May be fixed by #10142
Assignees
Labels
BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch enhancement
Milestone

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants