vite-plugin-relay icon indicating copy to clipboard operation
vite-plugin-relay copied to clipboard

Support Vite 3 and latest babel-plugin-relay

Open tobias-tengler opened this issue 1 year ago • 12 comments

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 avatar Aug 12 '22 10:08 tobias-tengler

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

tobias-tengler avatar Aug 12 '22 10:08 tobias-tengler

Feel free to!

oscartbeaumont avatar Aug 12 '22 10:08 oscartbeaumont

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

tobias-tengler avatar Aug 12 '22 11:08 tobias-tengler

@oscartbeaumont Okay tests should be fixed :)

tobias-tengler avatar Aug 12 '22 12:08 tobias-tengler

@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 avatar Aug 12 '22 13:08 tobias-tengler

@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 avatar Aug 14 '22 11:08 tobias-tengler

@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 :)

tobias-tengler avatar Aug 15 '22 13:08 tobias-tengler

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 avatar Aug 15 '22 14:08 oscartbeaumont

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.

oscartbeaumont avatar Aug 15 '22 14:08 oscartbeaumont

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 avatar Aug 15 '22 15:08 tobias-tengler

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

tobias-tengler avatar Aug 20 '22 12:08 tobias-tengler

@oscartbeaumont bump

tobias-tengler avatar Aug 25 '22 17:08 tobias-tengler

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

tobias-tengler avatar Sep 02 '22 08:09 tobias-tengler

@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?

tobias-tengler avatar Sep 08 '22 14:09 tobias-tengler

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 avatar Sep 10 '22 05:09 oscartbeaumont