purescript / trypurescript

PureScript in the browser
https://try.purescript.org
BSD 3-Clause "New" or "Revised" License
119 stars 50 forks source link

Update to 0.15.0; use ES Module Shims #275

Closed JordanMartinez closed 2 years ago

JordanMartinez commented 2 years ago

Description of the change

Fixes #274 and fixes #263. Another take that succeeds #274.

Original opening thread In #274, I experienced a few issues that made me rethink how I added initial support for `0.15.0`. I can explain in a future comment because it's late right now. Regardless, this PR is currently stuck due to this error: > Uncaught (in promise) DOMException: Permission denied to access property "_$s" on cross-origin object [srcdoc:1]() Clicking on `srcdoc:1` leads to this message: > Error loading this URI: Unknown source I'm baffled on how to proceed. I've been using Firefox to test this out, so it may be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1721891. When I tried it on Chromium, the error I get is this: > Uncaught (in promise) DOMException: Blocked a frame with origin "null" from accessing a cross-origin frame. > at about:srcdoc:1:163 Clicking on `srcdoc:1:163` displays `No resource with given URL found`.

Changes made:


Checklist:

JordanMartinez commented 2 years ago

Turns out the above error was a false alarm. We use index.html to create an iframe using frame.html as the source. es-module-shims appears to create an iframe within the head of frame.html. That's the source of the above error. I'm not sure why it does that, but it distracted me.

The actual reason why the code wasn't working is because there wasn't a wrapper calling main(); in the script's text. Adding that makes the example appear.

JordanMartinez commented 2 years ago

And I think the above error was fixed in a more recent version of es-module-shims. I was using 1.5.1, but it no longer appears in 1.5.5.

mikesol commented 2 years ago

Going through this now. I was able to get files served by running npx http-server --cors (we should probably update the readme, as it currently recommends npx http-server).

After that, I'm getting:

VM60 about:srcdoc:1 Uncaught (in promise) DOMException: Blocked a frame with origin "null" from accessing a cross-origin frame.
    at about:srcdoc:1:163

Is that where you're at as well?

JordanMartinez commented 2 years ago

Going through this now. I was able to get files served by running npx http-server --cors (we should probably update the readme, as it currently recommends npx http-server).

After that, I'm getting:

VM60 about:srcdoc:1 Uncaught (in promise) DOMException: Blocked a frame with origin "null" from accessing a cross-origin frame.
    at about:srcdoc:1:163

Is that where you're at as well?

See the package.json file. If you run npm i && npm run serve:dev, it'll run the code properly. The DOMException is what I get, too, but it's something to ignore. On my side, the code does get run if I'm not using the shims.

AFAICT, the shims don't currently work because they aren't imported in the code we're using. Put differently, this should work:

import React from 'react';
..

but this doesn't work

// main import script
import * as React from "../React/index.js";
...

where React/index.js is:

import React from 'react';
...

I think the solution there are two possible solutions:

For now, I'm going with the esbuild option since that seems to be what #264 is for.

mikesol commented 2 years ago

Would it be the case, then, that esbuild bundles and sends the whole application with each call to compile?

natefaubion commented 2 years ago

AFAICT, the shims don't currently work because they aren't imported in the code we're using. Put differently, this should work:

What convinced you that this is the case? It's not clear to me whether the imports are transitive should matter. Is this documented?

JordanMartinez commented 2 years ago

Code now works, but the main function gets called infinitely.

natefaubion commented 2 years ago

Doing a full bundle on every compile is extremely expensive, both on the server and for the user to pull down. I personally think it's a non-starter.

natefaubion commented 2 years ago

I'm pretty sure #264 is for only bundling the UI, not in general for all code the user inputs.

natefaubion commented 2 years ago

add another import map for all packages in the package set to frame.html

I don't think you need to add an import map for all packages in the package set? You only need it for specific packages that import specific foreign modules. This seems fine to me, and more preferable than using esbuild.

JordanMartinez commented 2 years ago

AFAICT, the shims don't currently work because they aren't imported in the code we're using. Put differently, this should work:

What convinced you that this is the case? It's not clear to me whether the imports are transitive should matter. Is this documented?

Let's say I want to run the following code that uses the decimal.js shim:

module Main where

import Prelude

import Data.Foldable (fold)
import Effect (Effect)
import Effect.Console (log)
import TryPureScript as TP
import Data.Decimal as D

main :: Effect Unit
main = TP.render =<< TP.withConsole do
  log $ show $ D.fromString "2424.24245238523752384034"

The unbundled JS returned is:

import * as Control_Bind from "../Control.Bind/index.js";
import * as Data_Decimal from "../Data.Decimal/index.js";
import * as Data_Maybe from "../Data.Maybe/index.js";
import * as Data_Show from "../Data.Show/index.js";
import * as Effect from "../Effect/index.js";
import * as Effect_Console from "../Effect.Console/index.js";
import * as TryPureScript from "../TryPureScript/index.js";
var main = /* #__PURE__ */ Control_Bind.bindFlipped(Effect.bindEffect)(TryPureScript.render)(/* #__PURE__ */ TryPureScript.withConsole(/* #__PURE__ */ Effect_Console.log(/* #__PURE__ */ Data_Show.show(/* #__PURE__ */ Data_Maybe.showMaybe(Data_Decimal.showDecimal))(/* #__PURE__ */ Data_Decimal.fromString("2424.24245238523752384034")))));
export {
    main
};

// I add this part so the code actually runs.
main();

If I remap those imports from ../Module.Path/index.js to ./js/output/Module.Path/index.js, and then update frame.html to the following:

<!DOCTYPE HTML>
<html>
<head>
  <title>Try PureScript!</title>
  <meta content="text/html;charset=utf-8" http-equiv="Content-Type">
  <meta content="utf-8" http-equiv="encoding">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <script>
    window.esmsInitOptions = {
      // -- Hooks --
      // Module load error
      onerror: (e) => {
        console.log("Error while loading module: ");
        console.log(e);
        throw e;
      },
    };
  </script>
  <!--
    JSPM Generator Import Map
    Edit URL: https://generator.jspm.io/#Y2NnYGCzD80rySzJSU1hSMpM183MK0lNTy1yMNQz0zM1ZEhJTc7MTczRyyp2MDTQM9YzZChKTUwu0U3Jz3UwNNMzxCqiX5xaVJZaBJGAKystzUxxsACaYQQAoBlP83cA
  -->
  <script type="importmap">
  {
    "imports": {
      "big-integer": "https://ga.jspm.io/npm:big-integer@1.6.51/BigInteger.js",
      "decimal.js": "https://ga.jspm.io/npm:decimal.js@10.3.1/decimal.js",
      "react": "https://ga.jspm.io/npm:react@16.13.1/index.js",
      "react-dom": "https://ga.jspm.io/npm:react-dom@16.13.1/index.js",
      "react-dom/server": "https://ga.jspm.io/npm:react-dom@16.13.1/server.browser.js",
      "uuid": "https://ga.jspm.io/npm:uuid@8.3.2/dist/esm-browser/index.js"
    },
    "scopes": {
      "https://ga.jspm.io/": {
        "object-assign": "https://ga.jspm.io/npm:object-assign@4.1.1/index.js",
        "react": "https://ga.jspm.io/npm:react@16.14.0/index.js",
        "scheduler": "https://ga.jspm.io/npm:scheduler@0.19.1/index.js"
      }
    }
  }
  </script>

  <!-- ES Module Shims: Import maps polyfill for modules browsers without import maps support (all except Chrome 89+) -->
  <script async src="https://ga.jspm.io/npm:es-module-shims@1.5.1/dist/es-module-shims.js" integrity="sha384-ZGtUNdKMtA2sSrO9dN9d2TOeiSlXlDSrOrasgO9YoKR4LsDV7RLdUvvX1M0gCkl2" crossorigin="anonymous"></script>

  <link rel="modulepreload" href="https://ga.jspm.io/npm:big-integer@1.6.51/BigInteger.js" integrity="sha384-Lxn1K4ox8//F5O+uihYrTIJvbfM+UJaRL2SdFA2s/z0v6i5TrWg9B8RzfjM60R9y"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:decimal.js@10.3.1/decimal.js" integrity="sha384-camBDJB1KoavtyRdQfdzzR4mj+uAKnNlvseJIwqKYOaHWnjZmnXR+1vyBP1WlZd0"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:object-assign@4.1.1/index.js" integrity="sha384-iQp1zoaqIhfUYyYkz3UNk1QeFfmBGgt1Ojq0kZD5Prql1g7fgJVzVgsjDoR65lv8"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react-dom@16.13.1/cjs/react-dom-server.browser.production.min.js" integrity="sha384-XL5bjsaJ5UPjTIPsyzUfscq41OFbVXsLojl53DCgFfdzKNNUL2MOSd0WM/upjd53"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react-dom@16.13.1/cjs/react-dom.production.min.js" integrity="sha384-HDQjgsF+F2j5XuiiDtCiuLA1vgyHr9ONiNrBhKmJEVa43KqIZdAB7VN2+QjUGwvq"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react-dom@16.13.1/index.js" integrity="sha384-zAu0L7n/xnRAG7+D/qd1K4E1/2tYK4hL3puH/5YEliHNnO3OOt5jUhnnemFTHmIG"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react-dom@16.13.1/server.browser.js" integrity="sha384-dsW+4a3rjWELxqtR1rVEIeLNHGyMjRFwdBVz6OG30oBLs8tO2/dYUIxEvc515PSZ"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react@16.13.1/index.js" integrity="sha384-T0mNrKFgmKBye+m7XrpShrh+7kfix2zQd9qrR8xytnppcxESpgzrb4IJXvE+uJcl"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react@16.14.0/cjs/react.production.min.js" integrity="sha384-pTMZhybzHZ+1G029kWUmoGvTrBp1C+2oJAkZV48BBq7+e6Hk3bGuXtvxT2vQfBqj"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:react@16.14.0/index.js" integrity="sha384-jVagjV+2YtlseazU2byX6gMLPHaA5Ps2c6HhvsGDlWjn45YCCoU1q+QtQTOb1MjT"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:scheduler@0.19.1/index.js" integrity="sha384-HIW1B3OQdGPkUEgh29MiYoUKQp+mFTjw50hRVvGhZgUaNYrUFYN07a+3CqC0/I7L"/>
  <link rel="modulepreload" href="https://ga.jspm.io/npm:uuid@8.3.2/dist/esm-browser/index.js" integrity="sha384-y7UmGnwe+/V/S4AD5T6MLU9dHjA3PLvuVGNhrqZApiNbq9Lf8s3aNRrcnzOa+qtR"/>
-  <script src="js/frame.js"></script>
</head>
<body>
  <main id="main"></main>
+  <script>
+    import * as Control_Bind from "./js/outpControl.Bind/index.js";
+    import * as Data_Decimal from "./js/outpData.Decimal/index.js";
+    import * as Data_Maybe from "./js/outpData.Maybe/index.js";
+    import * as Data_Show from "./js/outpData.Show/index.js";
+    import * as Effect from "./js/outpEffect/index.js";
+    import * as Effect_Console from "./js/outpEffect.Console/index.js";
+    import * as TryPureScript from "./js/outpTryPureScript/index.js";
+    var main = /* #__PURE__ */ Control_Bind.bindFlipped(Effect.bindEffect)(TryPureScript.render)(/* #__PURE__ */ TryPureScript.withConsole(/* #__PURE__ */ Effect_Console.log(/* #__PURE__ */ Data_Show.show(/* #__PURE__ */ Data_Maybe.showMaybe(Data_Decimal.showDecimal))(/* #__PURE__ */ Data_Decimal.fromString("2424.24245238523752384034")))));
+    export {
+        main
+    };
+    main();
  </script>
</body>
</html>

I get the following error in the console:

Uncaught SyntaxError: import declarations may only appear at top level of a module

and the position of the error is the first import:

import * as Control_Bind from "./js/outpControl.Bind/index.js";

You only need it for specific packages that import specific foreign modules.

If only a few need to be in the import map, that would be preferable than creating an import map for the entire package set.

natefaubion commented 2 years ago

For modules you need <script type="module">. I wonder if we can avoid rewriting paths by using a <base> tag.

JordanMartinez commented 2 years ago

For modules you need <script type="module">.

:facepalm: Wow... That change fixed it. And the import maps didn't need to be updated.

EDIT: actually, setting the script's type to module was what I was already doing in the frame.js code. Hm...

JordanMartinez commented 2 years ago

I wonder if we can avoid rewriting paths by using a tag.

Not quite. Since this is used in both dev (./js/output/ignored) and production settings , hard-coding it for dev breaks it for production.

I tried excluding the base element from frame.html and adding it in frame.js. However, I got loading errors. I think the issue is that es-module-shims creates a closure where relative URLs will resolve to what the 'base' element is at that time, not what it is later once the base element gets added.

If I'm wrong or you've got a better idea for how to fix this, let me know.

JordanMartinez commented 2 years ago

This is ready for a final review. Code builds and ES modules work.

natefaubion commented 2 years ago

Not quite. Since this is used in both dev (./js/output/ignored) and production settings , hard-coding it for dev breaks it for production.

Sorry can you clarify? dev and production use different path schemes? Is that necessary? If so, is there a way to configure it at build time, or app launch time (not necessarily client-side)? If there is a reasonable way to avoid a search and replace, that would be nice.

JordanMartinez commented 2 years ago

Sorry can you clarify? dev and production use different path schemes? Is that necessary? If so, is there a way to configure it at build time, or app launch time (not necessarily client-side)? If there is a reasonable way to avoid a search and replace, that would be nice.

We sort of already configure it at build time via the Config.purs file:

If we wanted to do it without a search and replace approach and use the base element, then we'd need to overwrite that value. Otherwise, it would require duplicating the frame.html file.

I've pushed a commit that uses an NPM script to overwrite it to the correct values.

JordanMartinez commented 2 years ago

Can this PR get an approval? I believe I've addressed all feedback.