Revert shim mode

[JordanMartinez]

Description of the change

Reverts #295. See #294 (comment) and I'm still waiting for Andy to confirm.

---

Checklist:

• Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0 by @)")
• Linked any existing issues or proposals that this pull request should close
• Updated or added relevant documentation
• Added a test for the contribution (if applicable)

[andys8]

Issues are very likely depending on browser and version. So, would be good to add that information. I'll have a look tomorrow and compare browsers in combination with es-module-shims settings.

[JordanMartinez]

Issue is on Firefox 103.0.1 on a Linux OS. I haven't checked on Chrome.

[andys8]

Poor man's browserstack testing 😉

Browsers

• Top Left: Chromium 104
• Bottom Left: Qutebrowser 2.5.2 based on Chromium 87
• Top Right: Firefox 103
• Bottom Right: Pale Moon 32 based on Firefox

Example 1

module HelloReactHooks.Main where

import Prelude

import Data.Maybe (Maybe(..))
import Effect (Effect)
import Effect.Exception (throw)
import React.Basic.DOM as R
import React.Basic.DOM.Client (createRoot, renderRoot)
import React.Basic.Hooks (Component, component)
import Web.DOM.NonElementParentNode (getElementById)
import Web.HTML (window)
import Web.HTML.HTMLDocument (toNonElementParentNode)
import Web.HTML.Window (document)

main :: Effect Unit
main = do
  doc <- document =<< window
  root <- getElementById "root" $ toNonElementParentNode doc
  case root of
    Nothing -> throw "Could not find root."
    Just container -> do
      reactRoot <- createRoot container
      app <- mkApp
      renderRoot reactRoot (app {})

mkApp :: Component {}
mkApp = do
  component "App" \_ -> React.do
    pure (R.text "Hello!")

Example 2

module Main where

import Prelude

import Data.Foldable (fold)
import Effect (Effect)
import TryPureScript (h1, h2, p, text, list, indent, link, render, code)

main :: Effect Unit
main = render $ h1 $ text "a"

Example 1, shimMode: false

Example 2, shimMode: false

Example 1, shimMode: true

Example 2, shimMode: true

Example 1, import uuid from 'uuid'

Example 2, import uuid from 'uuid'

Example 1, no settings, no import (likely same as shimMode: false)

Example 2, no settings, no import (likely same as shimMode: false)

Summary

• Example 1 (react) never worked in Chromium 87 (qutebrowser). What is necessary to make it work (main question to focus on)?
• shimMode: true can break otherwise working Firefox and Chrome browsers
• Is Example 2 a valid double reproducer for the double rendering we saw in Firefox? I'm surprised I didn't see it happening

Further investigation

There are two errors.

Here people claim this one is expect:

Failed to resolve module specifier "react-dom". Relative references must start with either "/", "./", or "../"." is expected.

[andys8]

Solution (?)

Updating es-module-shims to 1.5.12

No shim mode, no settings, no import

<script async src="https://ga.jspm.io/npm:[email protected]/dist/es-module-shims.js" crossorigin="anonymous"></script>

Example 1

Example 2

Summary

Updating es-module-shims to 1.5.12 seems to be a valid solution for the issues I could reproduce.

Additional safety?

Having said that, I'm a little concerned that I wasn't able to duplicate the double rendering problem. In case it's possible to reproduce it somehow (after the update), I would recommend adding an additional import script, in order to force the polyfills to kick in (what is described here: "The first import being a bare specifier in the pattern above is important to ensure this.")

  <script type="module">
    import uuid from 'uuid';
  </script>

The README example imports react and logs it. To me this looks weird, and I'd prefer to avoid it because the logs including errors are confusing for the users. But if necessary, I would just do it and add a comment to the implementation, instead of further spending time on this issue.

[andys8]

Comparing es-module-shims versions

No import, no shimMode, Example 1

1.5.1

1.5.9

1.5.12 ✔️

1.5.13

1.5.14

Summary

"Solution" is to pin to 1.5.12 and add a comment to only update if tested.

[natefaubion]

Is it worth opening a bug ticket and asking for a recommendation?

[andys8]

@natefaubion Opened an issue guybedford/es-module-shims#324 (not the best description, but I'm running out of time) 😉

[andys8]

I suggest to go with 1.5.12 for now: #298

See guybedford/es-module-shims#324 (comment)

[andys8]

Update

I'm pretty sure the the issues with the versions 1.5.13 - 1.5.16 are related to this code.

trypurescript/client/public/js/frame.js

Lines 13 to 14 in 3cdcc8f

	}, { once: true });
	}, { once: true });

The event listeners are initialized with once: true. I think what we see with the browsers with issues (e.g. latest firefox) is that the import machinery of es-module kicks in, and triggers re-initialization, after polyfills are applied. Because of this flag it's not executed a second time. Removing these will execute the inner function twice for polyfilled browsers, but it'll show up and work for the user (tested with 1.5.16), although double execution and import errors showing up multiple times in the log.

There is also a noLoadEventRetriggers setting in es-module-shims, but I couldn't see any effect or difference for this project, when I tried it.

Demo

• es-module-shims 1.5.16
• Removed both once: true flags for the event listeners
• Tested with react and standard example

screencast-2022-08-16_00.21.50.mp4

• One can see the double message event (log)
• One can see the "expected" polyfill error, but for each change
• There is another unexpected error regarding Foldable for the standard example
• Note that - ignoring the logs - the application and content is working for the user

[natefaubion]

Can we serve a noop import map module and always import that first, if it fixes the double load issue?

[andys8]

Can we serve a noop import map module and always import that first, if it fixes the double load issue?

Tried importing a library of the ones in the importmap, didn't make a difference when testing with react examples (since they also have an import on top). I assume this is what you mean with "noop import map"?

• Also tried async or not on several scripts
• Different order of scripts in frame.html
• Different options of es-module-shims
• Changing frame.js to being a module
• Changing frame.js to import one of the libraries
• Manually making sure event listeners either get removed or are only set once

[JordanMartinez]

Superceded by #298