search

brython-dev / brython

Brython (Browser Python) is an implementation of Python 3 running in the browser
BSD 3-Clause "New" or "Revised" License
6.39k stars 510 forks source link

https://cdn.jsdelivr.net/npm/brython@3/brython.min.js currently broken #1806

Closed throwaway-a closed 3 years ago

throwaway-a commented 3 years ago

I've got multiple people saying they can't use my code which depends on Brython. It imports it via:

<head>
--
  | <meta charset="utf-8">
  | <script type="text/javascript"
  | src="https://cdn.jsdelivr.net/npm/brython@3/brython.min.js">
  | </script>
  | <script type="text/javascript"
  | src="https://cdn.jsdelivr.net/npm/brython@3/brython_stdlib.js">
  | </script>
  | </head>

The error, from inspector console view in Chrome

--errSyntaxError: Invalid or unexpected token
brython.js:9583 Uncaught SyntaxError: Invalid or unexpected token
    at $make_exc (brython.js:9582)
    at brython.js:9603
    at brython.js:9288
brython.js:6277 Uncaught TypeError: Cannot read properties of undefined (reading 'VFS')
    at Object.$B.set_import_paths (brython.js:6277)
    at Object.$B.parse_options (brython.js:6301)
    at $B.parser.brython (brython.js:6326)
    at onload (test.html:13)
PierreQuentel commented 3 years ago

Many thanks for reporting this, it's scary... There is apparently a bug in the minification program used by jsdelivr (same for cdnjs) when transforming brython.js to brython.min.js.

Until the bug is fixed, the best is to use brython.js. I have modified the README accordingly.

The difference in size of the compressed files is only 20 kB.

MartinKolarik commented 3 years ago

Hi, Martin from jsDelivr here. It really seems like a terser issue. If you'd like to get it fixed, I suggest reporting it at their issue tracker. I'm not familiar enough with brython to help in that regard but if a fix is released, I'll make sure we deploy it quickly.

PierreQuentel commented 3 years ago

Thanks Martin for the fast reaction.

I think I have found the code in Brython that causes the minification error. I will publish a new Brython release tomorrow and I will report an issue on the Terser tracker.

PierreQuentel commented 3 years ago

I have just released version 3.10.3, after installing terser locally (version 5.9.0) and testing that all the tests in Brython test suite pass with the version of brython.min.js produced by terser.

Unfortunately, the version generated by jsDelivr from brython.js, while loading without error this time, does not pass all the tests...

This is really annoying. @MartinKolarik, it is possible that jsDelivr makes brython.min.js the same as brython.js until this issue is solved ?

For the next releases, should I include the version of brython.min.js that I can test locally in the npm package, or would it be erased by the one generated by jsDelivr scripts ?

MartinKolarik commented 3 years ago

You can certainly ship your own min file and we'll use it without any modification in that case. I'll also try to update our terser to 5.9 today.

MartinKolarik commented 3 years ago

We replaced the 3.10.3 file by a version built with Terser v5.9.0.

PierreQuentel commented 3 years ago

Thanks Martin. The version produced by v5.9.0 has the same issue as with 5.7.1, and I am almost sure I know why. In a few words: Brython generates a set of Javascript functions based on a template function with code like

function f(){
  // placeholder
 ...
}

other_function = (f + '').replace('// placeholder', <some code>)

The problem is that the minifier changes the source code of function f() and the line // placeholder is removed...

I have changed this code to avoid the issue. If possible I would like to avoid releasing another version ; if I send you a working version of brython.min.js can you put it on the CDN server ?

MartinKolarik commented 3 years ago

@PierreQuentel I see, that's definitely a problem for the typical minifier setup. I think for the next versions, it'll be best if you ship your own minified version. For 3.10.3 if you send me a fixed version, I can replace it.

PierreQuentel commented 3 years ago

@MartinKolarik I have sent it by email. Thanks for your help !

MartinKolarik commented 3 years ago

@PierreQuentel did you send it to the address in my GH profile? Don't see anything from you. Maybe push it to a branch in this repo?

PierreQuentel commented 3 years ago

Yes, I sent it at this address, and didn't get a non-delivery notification. It seems that someone up above put a spell on this release :-)

I have pushed it to this repo at www/src/brython.min.js.

MartinKolarik commented 3 years ago

Ok, it should finally work now.

PierreQuentel commented 3 years ago

Yes, apparently everything works now !

@throwaway-a can you confirm and close the issue if it is the case ?

PierreQuentel commented 3 years ago

I am closing the issue. @throwaway-a don't hesitate to reopen it if appropriate.