Repository navigation
Polyfill is not defined when using spago bundle-app #34
Description
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.jsExpected 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.
Activity
JordanMartinez commentedon Aug 31, 2021
Which version of
pursare you using to produce this?jamesdbrock commentedon Aug 31, 2021
Thanks for the bug report @ozkutuk ! Do you have any suggestions about how this should be solved? Do you think we should export the
polyFillfunction fromTyped.js?jamesdbrock commentedon Aug 31, 2021
purescript-arraybuffer/src/Data/ArrayBuffer/Typed.js
Line 20 in 20f4de1
JordanMartinez commentedon Aug 31, 2021
Reason why I ask is because of this issue: purescript/purescript#4044
jamesdbrock commentedon Aug 31, 2021
The PR which @JordanMartinez mentioned purescript/purescript#4044 was merged and included in
purs14.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.3and I got the expected output:purescript-arraybuffer wasn’t in package-sets for any
purs14.x versions prior to 14.3. What version ofpursdid you try this with @ozkutuk ?ozkutuk commentedon Aug 31, 2021
Weird,
purs --versionreports0.14.4. Additionally,spago version == 0.20.3and package-set ispsc-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.ozkutuk commentedon Aug 31, 2021
@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.
ozkutuk commentedon Aug 31, 2021
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.hsseems pretty self-contained, so I may find out something. Now, it seems apparent that this not related toarraybuffer, 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-fledgedTypedArraypolyfill either, so I think it would be a good idea to remove it anyway.jamesdbrock commentedon Sep 1, 2021
Oh yeah you're right purescript/purescript#4044 was introduced in
purs14.4 https://github.com/purescript/purescript/blob/master/CHANGELOG.md#v0144So your test
purs14.3purs14.4purs14.4 minus Add support for bundling function declarations purescript/purescript#4044JordanMartinez commentedon Sep 1, 2021
Thanks all for the analysis!
jamesdbrock commentedon Sep 8, 2021
Here's another way to see this same bug.
In a clone of
purescript-arraybuffer:jamesdbrock commentedon Sep 8, 2021
Here's some archeology
5200641
f224bbf
#23 (comment)
jamesdbrock commentedon Sep 21, 2021
We should try to come up with a solution in this library which doesn't depend on waiting for a new
pursversion.Delete the TypedArray polyfill
Delete the TypedArray polyfill
JordanMartinez commentedon Sep 21, 2021
For context, https://caniuse.com/mdn-javascript_builtins_typedarray
JordanMartinez commentedon Sep 21, 2021
And here's all methods: https://caniuse.com/?search=typedarray
JordanMartinez commentedon Sep 21, 2021
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
affjaxdoes in requiring users to bring their ownxhr2for the library to work.In other words, we would
Delete the TypedArray polyfill
jamesdbrock commentedon Sep 21, 2021
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 withpurs bundlenow, I hope this solves your problem.JordanMartinez commentedon Sep 21, 2021
@jamesdbrock Why wasn't this published as a breaking release (e.g.
v12.0.0)?jamesdbrock commentedon Sep 21, 2021
Yeah, it should be a breaking release.
JordanMartinez commentedon Sep 22, 2021
I believe fixing this would mean doing the following:
v11.0.3, which is the same asv11.0.1v12.0.0Ideally, we could avoid this work by just checking out
v11.0.1and publishing that tag asv11.0.3, but I'm not sure if that works or not.jamesdbrock commentedon Sep 22, 2021
Ok I'll do that.