Erroneous build config causes Presentation failure on updating packages in use

[narration-sd]

** Note on the update*

The description here is long, including the demanding rollback procedure if the error occurs, and I still had my own questions as to what the full picture was.

Thus I've worked out a description of what appears to be proper practice to clear up sanity-astro's problems (and for other packages' peerDependency handling??), and added it as a comment below, with a corrected package.json attached.

Describe the bug

In #2150 against @sanity/visual-editing, you can find a trail of woe where Presentation ceased to function in Astro, on a previous update of that package. Possibly 2.7.1 may be the breakage point..

However, with the improved package's version 2.8.0, the fix didn't get implemented so that Netlify or Vercel deployments would be repaired for Presentaion, when normally updating with sanity-astro, A contorted list of procedures has been found necessary to get the fix actually to operate in deployments. The apparent reason is in the mixture of config in the @sanity /astro package, which needs to be repaired, as will be described h ere.

To Reproduce

Presumption here is that you are building an app within the monorepo framework provided with the @sanity/astro package. It seems likely that the same error can occur when simply installing the @sanity/astro package in a different project framework.

1. have project as described
2. update to the fresh @sanity/visual-editing@2.8.0, in the package folder and in each app folder, using ncu -u so that other updated packages such as the client are all up-to-date
3. pnpm install from root, which installs in all folders
4. test using pnpm run dev, in the app. This works,, as Presentation visual editing can be seen functional.
5. now commit and push to the repo which is monitored by Netlify or Vercel so that it redeploys.
6. the new deployment will not show improvement, with Presentation still broken, as in 2.7.0 breaks Astro visual editing support visual-editing#2150.
7. create an alternative branch using the Astro Node adapter, so that it can be built as well as run preview locally
8. this local Node build will also fail Presentation operation, allowing local troubleshooting

Expected behavior

Any of the described builds should have employed the @sanity/visual-editing@2.8.0 fix, and run Presentation properly.

Screenshots
These are available for the failure mode, and the final fix running to be described here, in #2150.

Which versions of Sanity are you using?

All up-to-date, at the points described for failure and fix

What operating system are you using?
Windows 11 very up-to-date, in development mode for local builds due to links

Which versions of Node.js / npm are you running?

$ npm -v && node -v
10.2.4
v20.11.1

Additional context

The error in config has been located and demonstrated to fix Presentation with @sanity/visual-editing@2.8.0, on all three deployment platforms: Netlify, Vercel, and Node locally. Description follows.

• The problem occurs because:
  • there is a bogus peerDependency for @sanity/visual-editing in the @sanity/astro packages/sanity-astro package.json. This line is improper, because there is already the primary entry for visual-editing under dependencies. In fact, sanity-astro is the place @sanity/visual-editing is actually used in earnest for program code.
  • in the root folder of @sanity/astro monorepo, there are several questionable commands; one of these is auto-install-peers, which will cause the old version 2.7.1 of visual-editing to be installed along with the 2.8.0 set by ncu -u or manual package.json editing which does not notice the bogus peerDependency.
  • the result seems to be that pnpm build then connects the old version instead of the updated version when constructing the @sanity-astro package, and/or vite does so in linking the final executable result. Thus this result continues the 2.7.1 version's failures, and Presentation continues to fail, not showing blue outlines, not visual editing, and in fact not connecting the website pane with the editor under Presentation, with the noted console errors visible in the #2150 report.

As a first fix, the auto-install-peers was left alone in .npmrc, in case it had other purposes, and only the improper peerDependency on @sanity/visual-editing was removed from package.json in the monorepo packages/sanity-astro/package.json.

• it was also essential to produce a completely clean package-lock.yaml afterwards. A procedure is available for that under Slack discussion, involving clearing and recreation of package-lock.yaml itself, with all installs.
• this combination was sufficient to repair the problem, so that branches for Netlify, Vercel, and Node could then be committed and thus deployed, then demonstrated with their Presentation Visual Editing fully functional again.
• Vercel required a second manual redeploy from its web console, as described in the Slack item, to clear its on cache so that the corrected install for bulld would actually be done.

All of this is clearly a rather fraught procedure, thus it's suggested that a full fix be done on @sanity/astro, especially as there have been other such configuration errors which have caused great effort in the past.

• all of the above was done on monorepo uses of @sanity/astro, but because of the way pnpm/vite builds make the error, it seems likely the same failure will occur with projects only importing the package from npmjs.
• thus the fixes should not only be applied to source, but @sanity/astro should be published in a fresh version
• suggested fixes are:
  • clearly, the improper peerDependency for @sanity/visual-editing should be removed from package.json in packages/sanity-astro of the monorepo, as above demonstrates this as an immediate first fix
  • but also, a close examination should be done of root/.npmrc, and very likely at least the auto-install-peers command should be removed.
  • this .npmrc, as in past errors, appears that it may have been just a paste of boilerplate from a distant project, and so a person very familiar with pnpm should analyze and see if any of its commands are appropriate, of course the unnecessary removed, or that the removal of the entire .npmrc file itself may be best.

For any investigation uses, there are branches listed on a fork of @sanity/astro for each of the Netlify, Vercel, and Node deployments at success and showing some steps leading from fail to success, noted in the comment flow of sanity-io/visual-editing#2150.

[narration-sd]

Since the Slack listing of recovery procedures will go out of visibility by date, for later needs I'm reproducing it here.

The following presumes you are working in the monorepo, where the sanity-astro package is compiled as part of the app.

1. first, I'd simply remove the improper line from package.json in packages/sanity-astro. That's the reference to @sanity/visual-editing in the peerDependencies: section there. Besides causing problems, it's totally unnecessary.
2. then assure you're using not only the fresh visual-editing, but fresh other components that come with it. I use ncu -u, (install -g npm-check-updates), in the root, the package folder, and within any app folders you have, all of them crucial.
3. That step will update each package.json, and tell you what it's done. Very handy tool -- just doesn't handle peer dependencies.
4. now, critical deletions.
5. Delete package-lock.yaml from the root.
6. Delete node_modules from the root
7. Delete node_modules from packages/sanity-astro
8. Delete node_modules from any and all app folders that you have
9. there should be no node_modules findable anywhere in your project
10. then, from the root, do pnpm install. It will install in all folders, creating each node_modules fresh, and a fresh package-lock.yaml at root, with correct contents.
11. commit your project including all of this work
12. now you can push it to your repo on GitHub, or whatever you use
13. Netlify should start building a new deployment as usual from proper setup
14. once built, you can test, and in the Studio, Presentation should work(!)
15. you might have to shift-Refresh the browser, to assure it's downloaded the new website and studio, before the good results will show.

[narration-sd]

I've added this clarifying section, as it was bothering me further, as to:

• why the misconfiguration might have caused such serious result
• that there are not tools to manage upgrading ror peer dependencies.

Looking into those situations, and looking again at the packages/sanity-astro/package.json, then:

• one may note there is no dependencies listing
• instead, many items which should be dependencies are listed as devDependencies.
• having checked rules, many items are listed as peerDendencies which should not be listed so.

Thus, it's apparent the structure of dependencies isn't done in understanding, and should be much revised.

• those things the package directly uses should be dependencies: plain dependencies: libraries
• those things only used in development should be devDependencies: astro, sanity, vite, vite-plugin-dts
  • Vite might in some very few cases need to be a dependency, should be looked at, but appears to be its usual dev-only here, in spite of virtual module usage
• Only libraries required for use, but not set as dependencies, should be in peerDependencies. Thus most of those presently listed so are incorrect usage, and should be removed, to avoid the problems this issue troubleshoots
  • React and react-dom, however, are a special case...
  • react and react-dom need to be plain dependencies, as the package uses them
  • but react and react-dom need also to be peerDependencies. As the way to flag and thus prevent multiple React versions is for every usage of those libraries to declare their versions in peerDependencies. Failing to prevent multiple versions results in runtime React errors, due to mismateched contexts, etc..
  • this react/react-dom special practice work only if peers are defined for every package or app code where React is used. It also require manual attention to be sure the peerDemendencies for thses packages are always acxurate, up-todate..
  • auto-install-peers appears to be improper, in the .npmrc at project root, as it will exacerbate loading multiple versions for any config errors as discussed here

All right. That's complicated enough, but as simple as I can see to state it. The practice for React/react-dom may be the source of the confused configuration that has mixed in many improper peerDependencies may have been the idea that what's done for React ought to be standard practice.

Some of what I've read hints the pnpm may be particularly bad about handling mixed versions in peerDependencies. The note about .npmrc directives like auto-install-peers can be involved in this.

Rather than use a PR, I'll attach a suggested package.json for packages/sanity-astro here:

package.json

The answer to my question as to why package management tools skip this whole area, I think shows in the notes above:

• in most all cases, there should not be any package dependencies duplicated into peerDependencies
• if there's reason, as with React, it's a more complex issue, better handled with clear personal awareness for the case

[ChrisLaRocque]

Hey @narration-sd thanks for this and sorry it took so long to get a reply.

Looking at the reproduction steps in the initial message with the Vercel adapter, I'm unable to recreate. Are you able to recreate this issue somehow that I could take a look at?