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

docs(readme): adds a react-table-v6 package note #1576

Merged
merged 1 commit into from Oct 10, 2019

Conversation

kizu
Copy link
Contributor

@kizu kizu commented Oct 8, 2019

Creating this PR after a discussion at https://spectrum.chat/react-table/general/additional-package-just-for-the-v7~7115661c-7e60-4a61-a16f-4be7e6796349

TL;DR: right now the migration process from v6 to v7 can be complicated due to the v7 being completely different from v6, so at a larger codebase you could need to have both versions installed at the same time. While this is possible to do right now, by using an alias like npm:react-table@6.8.6, if you're using TypeScript things could become much more complicated as both packages would have the react-table as a module name.

My proposal is to push a legacy react-table-v6 package to the registry with the latest version of v6 and a changed module name in the TS definition. This would allow us to use both v6 and v7 versions at the same time without any problems just by replacing the old v6 version by this react-table-v6 package and then using the original react-table only for the v7 version.

While this can also be solved by a fork, I feel that given the v6 popularity this can be an issue for more people, and I feel that creating such package would be the best solution (and wouldn't need a lot of maintaining).

This PR adds a line to the README about this package, and, of course, should be merged only if this idea is accepted by the project's maintainers and after they would publish the corresponding package.

Thank you for react-table and the very promising v7 rewrite, it is an awesome project!

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 8, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4d25c1b:

Sandbox Source
naughty-agnesi-0l8mm Configuration

@tannerlinsley
Copy link
Collaborator

Would you be willing to add the TS changes need as well to this PR?

@kizu
Copy link
Contributor Author

kizu commented Oct 8, 2019

What do you think would be the best way to do this?

Basically, I feel this depends on if you'd want to push the potential future fixes to the v6 both into react-table and react-table-v6. If we'd want to have both, we probably would need to create a new branch just for this, where we could make the changes (basically, just the package.json and index.d.ts updates I believe?). Otherwise (if new updates would be ok only for the react-table-v6 package), I can create a PR into the v6 branch, but I feel that it would probably make sense to keep both?

@tannerlinsley
Copy link
Collaborator

Personally, I don't want to maintain updates to two separate locations. If we actually end up doing this, I would want a single location for maintenance releases to v6.

@kizu
Copy link
Contributor Author

kizu commented Oct 8, 2019

It is your call, but I think that this should happen quite rarely and shouldn't add almost any overhead . I personally would be ok with potential v6 updates being only in react-table-v6.

The cases when you'd need to have an update into the original react-table package for the v6 should be almost non-existent (only security-related ones would make sense?), and in this case making those manually without maintaining a separate branch should be ok?

If that's ok for you then, I'll go and create a PR into the v6 branch to change its package name, so you could then publish the new react-table-v6 from this branch?

@tannerlinsley
Copy link
Collaborator

tannerlinsley commented Oct 8, 2019 via email

@tannerlinsley tannerlinsley merged commit 720fb21 into TanStack:master Oct 10, 2019
@kizu
Copy link
Contributor Author

kizu commented Oct 10, 2019

@tannerlinsley Thanks! The only thing left I guess is for you to build and publish the v6 branch to the react-table-v6 package :)

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.

None yet

2 participants