Polyfill is not defined when using `spago bundle-app`

Describe the bug
polyFill function is not exported from Typed.js, but called at the top-level of Typed.js itself. This seems to cause issues with bundling, where the definition isn't picked up by the bundle, causing the top-level polyFill() call to report Uncaught ReferenceError: polyFill is not defined.

To Reproduce
Initialize a spago project via spago init. Add the following list of dependencies to spago.dhall:

  [ "arraybuffer"
  , "arraybuffer-types"
  , "console"
  , "effect"
  , "prelude"
  , "psci-support"
  ]

Modify src/Main.purs to be as follows:

-- src/Main.purs
module Main where

import Prelude

import Data.ArrayBuffer.Types (ArrayView, Int32)
import Data.ArrayBuffer.Typed as ArrayBuffer
import Effect (Effect)
import Effect.Console (log)

main :: Effect Unit
main = do
  _ :: ArrayView Int32 <- ArrayBuffer.empty 0
  log "hello"

Bundle it and run it via node:

$ spago bundle-app
$ node index.js

Expected behavior
See the output: "hello".

Actual behavior

<path-to-project>\index.js:6
  polyFill();
  ^

ReferenceError: polyFill is not defined

Additional context
Running the bundle through browser or node does not make a difference, failing in both cases.

Which version of purs are you using to produce this?

Thanks for the bug report @ozkutuk ! Do you have any suggestions about how this should be solved? Do you think we should export the polyFill function from Typed.js?

purescript-arraybuffer/src/Data/ArrayBuffer/Typed.js

Line 20 in 20f4de1

polyFill();

Reason why I ask is because of this issue: purescript/purescript#4044

The PR which @JordanMartinez mentioned purescript/purescript#4044 was merged and included in purs 14.3. Jordan suspects that that PR might have fixed the bug which you’re seeing.

I just tried your bug reproduction steps with purs 14.3 and I got the expected output:

$ node index.js 
hello

purescript-arraybuffer wasn’t in package-sets for any purs 14.x versions prior to 14.3. What version of purs did you try this with @ozkutuk ?

Weird, purs --version reports 0.14.4. Additionally, spago version == 0.20.3 and package-set is psc-0.14.4-20210826. I don't think it has any relevance but I have tried this on a Windows machine and the node version is v14.13.1.

@jamesdbrock It seems that purescript/purescript#4044 didn't make it into 0.14.3 release, so I am curious how this works on 0.14.3 at all. Maybe this was a regression that is present in the 0.14.4 release? I may downgrade the purs to 0.14.3 and try again, and report the results.

I have bundled both with purs 0.14.3 and 0.14.4, and indeed it bundles and runs as expected on 0.14.3. To be sure that this is caused by purescript/purescript#4044, first I have compiled purs compiler on v0.14.4 tag myself and then got a bundle from that. Later, I have reverted the commit introducing purescript/purescript#4044 and rebuilt purs again, then getting another bundle output from the resulting purs executable. Indeed, the bundle of the latter purs executable contains the function polyFill() declaration, while the former does not.

I will try to further analyze why this is the case. I am not really familiar with the purescript codebase but the Bundle.hs seems pretty self-contained, so I may find out something. Now, it seems apparent that this not related to arraybuffer, so should we re-open this issue on the purescript repo instead?

Regarding the polyfill itself, caniuse.com reports that TypedArrays are supported since IE10, and the polyfill in question is not a full-fledged TypedArray polyfill either, so I think it would be a good idea to remove it anyway.

Oh yeah you're right purescript/purescript#4044 was introduced in purs 14.4 https://github.com/purescript/purescript/blob/master/CHANGELOG.md#v0144

So your test

passes with purs 14.3
fails with purs 14.4
passes with purs 14.4 minus purescript/purescript#4044

Thanks all for the analysis!

Here's another way to see this same bug.

In a clone of purescript-arraybuffer:

spago -x spago-test.dhall build
purs bundle --main Test.Main --module Test.Main --output index.Test.Main.js output/**/*.js
node index.Test.Main.js

Here's some archeology

5200641

f224bbf

#23 (comment)

We should try to come up with a solution in this library which doesn't depend on waiting for a new purs version.

For context, https://caniuse.com/mdn-javascript_builtins_typedarray

And here's all methods: https://caniuse.com/?search=typedarray

As discussed in our call, we think a good way to handle this is to force users to handle the polyfill themselves. This is similar to what affjax does in requiring users to bring their own xhr2 for the library to work.

In other words, we would

update the readme to clarify that one must handle the polyfill themselves if they are working with older browsers
delete the polyfill
make a new release that is breaking

Hi @ozkutuk I took your advice and just deleted the polyFill() function. I also published a new v11.0.2 with the change. Thanks for making this issue! Whatever happens with purs bundle now, I hope this solves your problem.

@jamesdbrock Why wasn't this published as a breaking release (e.g. v12.0.0)?

@jamesdbrock Why wasn't this published as a breaking release (e.g. v12.0.0)?

Yeah, it should be a breaking release.

I believe fixing this would mean doing the following:

reverting the change
republishing a new v11.0.3, which is the same as v11.0.1
adding the change back in, except adding the readme update part, too
publishing a breaking release as v12.0.0

Ideally, we could avoid this work by just checking out v11.0.1 and publishing that tag as v11.0.3, but I'm not sure if that works or not.

I believe fixing this would mean doing the following:

reverting the change

republishing a new v11.0.3, which is the same as v11.0.1

adding the change back in, except adding the readme update part, too

publishing a breaking release as v12.0.0

Ideally, we could avoid this work by just checking out v11.0.1 and publishing that tag as v11.0.3, but I'm not sure if that works or not.

Ok I'll do that.