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

Bugfix for extended DataProvider cloning #347

Conversation

DevopsElectricJukebox
Copy link

@DevopsElectricJukebox DevopsElectricJukebox commented May 9, 2019

(This PR replaces #346)

It is not possible to use cloneWithRows(items) method on a class which extends DataProvider, the method will create an instance of the base class and not the extended class. This has been remedied by providing a separate clone() method which the subclass can re-implement. clone() is then called in cloneWithRows(items) instead of new DataProvider.

…class

adds new method clone() which subclasses must re-implement
@sbearben
Copy link

Would be great if this could get merged - was just working on a PR to do the same thing before I came across this

@@ -24,6 +24,11 @@ export default class DataProvider {
this.getStableId = (index) => index.toString();
}
}

public clone(): DataProvider {

Choose a reason for hiding this comment

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

Would it make sense for this method to be protected?

@naqvitalha
Copy link
Collaborator

@sbearben @DevopsElectricJukebox This fix is already in master. Instead of extending from DataProvider extend from BaseDataProvider and override newInstance. Closing.

@naqvitalha naqvitalha closed this Jul 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants