elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Chrome extensions (Grammarly in our case) cause runtime errors #182

Open erlandsona opened 2 years ago

erlandsona commented 2 years ago

This PR is a re-posting of this fix These patches fix the issue below.

Screen Shot 2022-06-10 at 8 01 24 AM

Here's the expanded stack trace.

Main.elm:3676 Uncaught TypeError: Cannot read properties of undefined (reading 'childNodes')
    at _VirtualDom_addDomNodesHelp (Main.elm:3676:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3684:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3684:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3684:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3684:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3670:1)
    at _VirtualDom_addDomNodesHelp (Main.elm:3684:1)
    at _VirtualDom_addDomNodes (Main.elm:3600:1)
    at _VirtualDom_applyPatches (Main.elm:3707:1)
    at Main.elm:4081:1
_VirtualDom_addDomNodesHelp @ Main.elm:3676
_VirtualDom_addDomNodesHelp @ Main.elm:3684
_VirtualDom_addDomNodesHelp @ Main.elm:3684
_VirtualDom_addDomNodesHelp @ Main.elm:3684
_VirtualDom_addDomNodesHelp @ Main.elm:3684
_VirtualDom_addDomNodesHelp @ Main.elm:3670
_VirtualDom_addDomNodesHelp @ Main.elm:3684
_VirtualDom_addDomNodes @ Main.elm:3600
_VirtualDom_applyPatches @ Main.elm:3707
(anonymous) @ Main.elm:4081
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
updateIfNeeded @ Main.elm:4615
requestAnimationFrame (async)
(anonymous) @ Main.elm:4626
sendToApp @ Main.elm:1885
(anonymous) @ Main.elm:1994
_Scheduler_step @ Main.elm:1810
_Scheduler_enqueue @ Main.elm:1784
_Scheduler_rawSpawn @ Main.elm:1715
(anonymous) @ Main.elm:5394
setInterval (async)
(anonymous) @ Main.elm:5394
_Scheduler_step @ Main.elm:1810
_Scheduler_enqueue @ Main.elm:1784
_Scheduler_rawSend @ Main.elm:1730
_Platform_dispatchEffects @ Main.elm:2102
_Platform_enqueueEffects @ Main.elm:2088
_Platform_initialize @ Main.elm:1889
(anonymous) @ Main.elm:4052
(anonymous) @ Main.elm:20
./elm/src/index.js @ index.js:19
__webpack_require__ @ bootstrap:19
__webpack_exec__ @ bootstrap.mackey.css:1
(anonymous) @ bootstrap.mackey.css:1
__webpack_require__.O @ chunk loaded:23
(anonymous) @ bootstrap.mackey.css:1
webpackJsonpCallback @ jsonp chunk loading:71
(anonymous) @ init-main.b89ac79a8ec35153daba.js:1

More info: https://github.com/elm/virtual-dom/pull/177#issuecomment-1125291829

Currently we've solved for this by patching virtual-dom with an outdated / seemingly unmaintained shell script call shelm.

Unfortunately, elm-json doesn't like the "locations" key that shelm depends on to do it's overriding so every time we install or update packages that "locations" key gets deleted and the developer has to remember to bring it back after an install.

This means recently we accidentally re-introduced this Grammarly bug into a production release.

Secondarily, using shelm means that our LSP tooling using ~/.elm gets out of sync with what shelm uses for our local packages sometimes breaking intellisense.

Merging this fix means

  1. we can remove the hacky shell script that's overriding this for us in our production builds and alleviate the incompatibility with other elm ecosystem tooling.
  2. It fixes potential issues with Grammarly for other people in the elm community that I know have been affected by this issue.
  3. The patch apparently fixes issues for other "Chrome Extensions" altho we don't have any users using any of the other extensions that might also be breaking elm/virtual-dom.
rupertlssmith commented 1 year ago

Do we have an idea of what classes of browser extension issues this patch will fix? The idea looks sound to me, just want to find out if this is a full or partial solution to the issue.