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

Support Vite 3 and latest babel-plugin-relay #424

Merged
merged 29 commits into from Sep 10, 2022

Conversation

tobias-tengler
Copy link
Collaborator

@tobias-tengler tobias-tengler commented Aug 12, 2022

I tried to use this plugin in a new Vite app and noticed that it doesn't work, due to the usage of require.resolve. While fixing this I also noticed that it has a dependency on a fixed version of babel-plugin-relay, which is kind of cumbersome to maintain, so I:

  • replaced require.resolve with just the plugin name
  • made babel-plugin-relay a peer dependency
  • forwarded the filename to transformSync in order to support artifactDirectory imports
  • updated all of the packages to their latest versions
  • changed the package type to "module" and used tsup to generate both commonjs and esm output
  • updated the README with more detailed instructions
  • added example project for Vite 3
  • switched to package linking instead of file:..
  • fixed an issue in the test pipeline

Closes #309
Closes #211

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont If it's alright for you, I would also credit myself in the README :)

@oscartbeaumont
Copy link
Owner

Feel free to!

@tobias-tengler
Copy link
Collaborator Author

Whoops. Forgot to update the tests. I'm on it!

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont Okay tests should be fixed :)

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont Sorry for the pings, but tests need to be run again :D I think the playwright installation issue should now also be fixed

@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented Aug 14, 2022

@oscartbeaumont if you approve the workflow again, the tests should now succeed!

I think it should be fine to go ahead and merge / release it.
It should be published as a major version bump, as it will break existing projects, if they do not have babel-plugin-relay installed in their project.

The migration path for users that are using v1 is:

  1. yarn add vite-plugin-relay@latest -D
  2. yarn add babel-plugin-relay@latest -D (The version should match the version of the installed Relay packages, but must be >= 13.0.1)
  3. Set eagerEsModule to true in your Relay configuration (Would already be required, if you are not using Typescript)

New users can also use my create-relay-app project to correctly setup Relay with this plugin in their projects.

@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented Aug 15, 2022

@oscartbeaumont I've re-added the Vite 2 example and changed the code to support both Vite 2 and 3. The pipeline should now run tests against both versions.

Users migrating now do not need to update their Vite version.

Could you review the code and give me feedback whether we can merge it or not? Would love to get this in asap :)

@oscartbeaumont
Copy link
Owner

Just give me till the end of this week to review. I wanna give it a proper test given so much has changed but I have been super busy.

@oscartbeaumont
Copy link
Owner

Thanks for all of your work by the way! I have been meaning to give this project some love but just haven't found the time for it.

@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented Aug 15, 2022

Okay great :)
I've just switched to package linking instead of file:./../.., since it causes the local cache to fill up quickly and took upwards of 5min sometimes for me to install the newest version.
So now when developing run yarn link in the root directory and then yarn link vite-plugin-relay in the example directories and they should always point to the current version of the code. I also updated the workflow to do the same.

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont Just wanted to remind you to review this, since it's the end of the week.

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont bump

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont Do you think you'll get a chance to review it this weekend?

@tobias-tengler
Copy link
Collaborator Author

@oscartbeaumont Sorry for being annoying, but it would be really great to get this in asap.
Would it help you if I broke down the changes into smaller PRs?

@oscartbeaumont
Copy link
Owner

oscartbeaumont commented Sep 10, 2022

I am really sorry for taking so long to review this. I know how annoying it is. Between my full-time job and being burnt out I just didn't get around to it until now.

I moved to a pnpm workspace to make installation quicker and not require any linking.

I have added @tobias-tengler as a GitHub contributor. When you create a GitHub release it will automatically push to npm. Would be nice to still use GH Issues/PRs to track changes before releases though. Also feel free to add me on Discord (oscartbeaumont#0004) if you wanna chat.

Edit: Before doing a GH Release you must update the version in the package.json's manually.

@oscartbeaumont oscartbeaumont merged commit 6d2abf6 into oscartbeaumont:main Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants