Switch from useCMEditViewDataManager() to useDocument()

[jackbuehner]

This PR finishes replacing usage of @strapi/helper-plugin. (@strapi/helper-plugin is deprecated and is not compatible with Strapi 5.) It builds on builds on #133. useCMEditViewDataManager() from @strapi/helper-plugin has no direct replacement, but the part used by this plugin is replaced by useDocument() from @strapi/admin/strapi-admin.

This PR also switches the <Button /> component usage to <LinkButton />. They look the same, but they allow right clicking to copy links and middle clicking to open in a new tab. If you click them, they behave exactly as the always have; event.preventDefault() is called and the regular logic takes over.

[mattmilburn]

Hi @jackbuehner I really appreciate the work you put into this PR! 🎉 I hate to break it to you 💔 that I've already handled a lot of this upgrade before seeing your PR, and I converted everything into TypeScript so it presents quite a conflict with your PR.

However, would you be interested in still rebasing this branch with the latest changes in PR 133 and migrating the useCMEditViewDataManager usage in EditViewRightLinks admin component? Also, I wouldn't mind if you wanted to rebase everything and force push the changes to avoid some ugly commit history.

I wonder if that implementation is going to change before the final v5 release but either way, it would be nice because I have not worked with migrating that functionality in a plugin yet 😄 Plus I don't want to steal your Github cred and this would be a good opportunity to be listed as a contributor.

I'm happy to discuss other improvements that might be needed too.

[jackbuehner]

No worries! I like the move to TypeScript. I would have posted about contributing on the PR, but I was in the zone on updating everything and just decided to do it.

I'm happy to rebase with your latest changes. It probably can't do it today, but I can definitely do it before the end of the weekend, if that is okay.

They said there shouldn't be too many more big changes now that we are in the release candidate phase, so hopefully this is how it will be. But who knows – they have not yet fully documented the replacements for useCMEditViewDataManager.

[jackbuehner]

Are you okay with switching Button to LinkButton? I can also save it for a different PR if you want to keep things separated.

[mattmilburn]

Are you okay with switching Button to LinkButton? I can also save it for a different PR if you want to keep things separated.

Oh sure, LinkButton is fine 👍🏻

[jackbuehner]

Sorry it took a little longer (I got sick). I just did a hard reset to your version of the strapi5 branch. I then committed on top of that. It should make it so this PR can be cleanly merged into mattmilburn:strapi5 Let me know if you need me to change anything!

[mattmilburn]

@jackbuehner Here are some additional things I found after running yarn build to test your updates locally. Do you mind adding these into your PR? Sorry the first item was something I missed before.

1. Add "styled-components": "^6.1.13" to both peerDependencies and devDependencies in package.json and run yarn.
2. In ListViewColumnProps and AddPreviewColumnProps can you use uid: UID.ContentType | undefined instead of uid: string.
3. In ListViewColumn, line 38, this hook can be simplified to usePreviewButton(layout.contentType.uid, data) to match your update to the hook param.

[mattmilburn]

@jackbuehner Also, have you tested your updates in a local Strapi app? When I configure the bare minimum config for a content type, I'm getting this error and I'm not sure what's causing it. Curious if you ran into this or not.

[jackbuehner]

@jackbuehner Also, have you tested your updates in a local Strapi app? When I configure the bare minimum config for a content type, I'm getting this error and I'm not sure what's causing it. Curious if you ran into this or not.

I did, but it was an earlier release candidate. Maybe something changed? I'll give it a look.

[jackbuehner]

The bundle has its own copy of @strapi/admin. This means that the useStrapiApp hook cannot find the StrapiApp context because they are separate. This was not a problem earlier in my PR because

You can verify that the bundled version is being used by adding console.log('WRONG CONTEXT') to the useContext function inside the createContext function in node_modules/@strapi/admin/dist/admin/Theme-frC82ceE.mjs:

function createContext(rootComponentName, defaultContext) {
  const Context = ContextSelector.createContext(defaultContext);
  const Provider = (props) => {
    const { children, ...context } = props;
    const value = React.useMemo(() => context, Object.values(context));
    return /* @__PURE__ */ jsx(Context.Provider, { value, children });
  };
  const useContext = (consumerName, selector) => ContextSelector.useContextSelector(Context, (ctx) => {
    console.log('WRONG CONTEXT')
    if (ctx)
      return selector(ctx);
    throw new Error(`\`${consumerName}\` must be used within \`${rootComponentName}\``);
  });
  Provider.displayName = rootComponentName + "Provider";
  return [Provider, useContext];
}

const [StrapiAppProvider, useStrapiApp] = createContext("StrapiApp");

It was not a problem earlier in my PR because I had used rollup and was not bundling .mjs files (which is what @strapi/admin was using). https://github.com/mattmilburn/strapi-plugin-preview-button/blob/43f794f46bb32ccd351f37493aee409c2030b1e2/rollup.config.mjs#L24C5-L26C8. I did this out of ignorance; I should have marked @strapi/admin as external.

strapi-plugin uses rollup behind the scenes. We need to set @strapi/admin as external with rollup, but I am not sure how we can do that with strapi-plugin build.

The code for build from strapi-plugin is at https://github.com/strapi/sdk-plugin/blob/c4702ad5c12fccc83530519a4c95814c09f17bdd/src/cli/commands/plugin/build.ts#L70C5-L75C8. The only options we can provide are the ones available via the command options at the bottom of the file.

Any ideas? @mattmilburn

[mattmilburn]

@jackbuehner Just an update - I'm reaching out to Strapi support on Discord. This is a popular plugin that even the Strapi team uses internally, so don't worry - we will get this figured out soon enough 🤞🏻

[jackbuehner]

Thanks @mattmilburn. My last thought was to use npx @strapi/sdk-plugin:init and try to add in bits of the code from this plugin until something breaks (and compare the package versions in package.json). I'll wait until you hear back from support on Discord.

[jackbuehner]

Might be related to strapi/strapi#21151

[danidoff]

Hey guys. I think they fix it in strapi 5.0.3

[mattmilburn]

Unfortunately I'm still seeing the same error after updating to v5.0.3.

[mattmilburn]

@jackbuehner This is possibly fixed in v5.0.5 🤞🏻

EDIT: Actually, false alarm. The issue is still happening in v5.0.5 😞

[SalahAdDin]

Not able to migrate plugins to version 5 yet!

[killthekitten]

Same issue in v5.3.0

[killthekitten]

@jackbuehner @mattmilburn there was an update from the Strapi team in this comment. It appears that the bug is only present when the plugin depends on an RC release of Strapi. Could you try updating the plugin to the latest Strapi?

In my comment above I might have only bumped the main project's dependency.

[mattmilburn]

This is still a problem in v5.4.0. I've created an issue with Strapi to describe the problem in case anyone is interested.

[mattmilburn]

@jackbuehner Problem solved - I was testing this by adding the plugin to a test application's package.json with file:../<filepath> and it's actually required to use yalc to link a plugin to the Strapi app. See this comment for extra details.

Merging this PR now and I'll cleanup the rest of the plugin details and get it ready to deploy v3 for Strapi 5 🚀 🎉

Also might be important to mention that at some point with Strapi v5, they will introduce an official preview feature that lets you do a lot more with previewing your editable content within Strapi itself. It looked quite robust when they demo'd it for me, which is exciting. Once that feature is released, I imagine this plugin will be obsolete ❤️

[jackbuehner]

@jackbuehner Problem solved - I was testing this by adding the plugin to a test application's package.json with file:../<filepath> and it's actually required to use yalc to link a plugin to the Strapi app. See this comment for extra details.

Dang... I was doing the same thing. Really wanted to avoid yalc! Thanks for your persistence in troubleshooting the issue you described to the Strapi team in #146.

[SalahAdDin]

@jackbuehner Problem solved - I was testing this by adding the plugin to a test application's package.json with file:../<filepath> and it's actually required to use yalc to link a plugin to the Strapi app. See this comment for extra details.

Dang... I was doing the same thing. Really wanted to avoid yalc! Thanks for your persistence in troubleshooting the issue you described to the Strapi team in #146.

yalc?