pluginpal / strapi-plugin-config-sync

:recycle: CLI & GUI for continuous migration of config data across environments
https://www.pluginpal.io/plugin/config-sync
MIT License
255 stars 37 forks source link

Issue with `react-diff-viewer-continued` #105

Closed bwp91 closed 1 year ago

bwp91 commented 1 year ago

Hey

Not an issue with this plugin per se, but building strapi fails on a new install with the config sync plugin due to:

https://github.com/Aeolun/react-diff-viewer-continued/issues/31

I needed to override the version in my package json like this:

  "dependencies": {
    "@strapi/admin": "4.14.4",
    "@strapi/plugin-documentation": "4.14.4",
    "@strapi/plugin-i18n": "4.14.4",
    "@strapi/plugin-users-permissions": "4.14.4",
    "@strapi/provider-upload-aws-s3": "4.14.4",
    "@strapi/strapi": "4.14.4",
    "strapi-plugin-config-sync": "^1.2.0",
  },
  "overrides": {
    "strapi-plugin-config-sync": {
      "react-diff-viewer-continued": "3.2.6"
    }
  },

for strapi to build successfully.

Maybe this will help someone else in the meantime until that ^ repo is fixed. Hopefully this issue can be closed soon!

boazpoolman commented 1 year ago

Hmm odd.

This plugin explicitly uses version 3.2.6 of the react-diff-viewer-continued as per the yarn.lock anyways. See: https://github.com/boazpoolman/strapi-plugin-config-sync/blob/master/yarn.lock#L4876

So I wouldn't know why that override would be necessary

boazpoolman commented 1 year ago

Also just two days ago a succesfull build of Strapi (with config-sync installed) can be witnessed in Github Actions. Using Strapi 4.14.4 and config-sync 1.2.0.

https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/6523104347/job/17713405843

bwp91 commented 1 year ago

Looks like the new version of react-diff-viewer-continued was release just two hours ago.

Maybe I had an issue because i use npm i to install rather than yarn?

boazpoolman commented 1 year ago

TIL: The yarn.lock is not published to NPM. See: https://www.npmjs.com/package/strapi-plugin-config-sync

That's just asking for touble IMO. That means if a dependency of this plugin doesn't have an explicit version constraint in the package.json I have 0 control over which version will be installed. I'm not sure what's good practice here. I'll have to do some research.

boazpoolman commented 1 year ago

Allright. Publishing .lock files is not really a thing.

That leaves us with 2 options:

  1. Wait for react-diff-viewer-continued to release a patch and in the meantime use your workaround
  2. Explicitly set the version constraint of react-diff-viewer-continued to 3.2.6 in the package.json of the plugin.

I'm gonna wait a bit to see if they'll come up with a patch any time soon. Otherwise I'll go with option 2 in the near future.

boazpoolman commented 1 year ago

I've locked the version of react-diff-viewer-continued to 3.2.6 in PR #107.

To prevent this from going unnoticed in the future I've updated the integration tests to install the plugin dependencies with the --no-lockfile flag. By doing that the dependency install job will behave just as it would if somebody would install the plugin from NPM (without a lockfile). And because of that we can replicate an issue like this in the pipeline. See: https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/6551531539/job/17792933173

boazpoolman commented 1 year ago

Released and fixed with version 1.2.1.