Re: Porting audio.js away from jQuery (Was: Re: MediaGoblin website moved to sourcehut pages (was: TLS certificate expired for mediagoblin.org, take 2))

[Andre Jaenisch]

Hey there,

short update (since I paused due to birth of my child).

I rebased on 522a61b24a1b7767682eaf7b29c59e40a0a9b73f /
https://git.savannah.gnu.org/cgit/mediagoblin.git/commit/?id=522a61b24a1b7767682eaf7b29c59e40a0a9b73f

You can see a comparison at
https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/compare/main...d2254dcdb0
(there's a bug in Gitea I guess, which fails over the slash in my
branch name - I'm replying here once I made progress and will update
the link).

Once I feel confident to wrap it up, I plan to squash the commits
(unless there is another process defined) to make it easier to
integrate the changes (as a single commit).

> Can you send me details of the failing tests please and the commands
> you used to run them?

I used ./runtests.sh but that failed with a cryptic message ($VENVS is
the place where I put all my Python projects. The git repo is cloned
into "src"):

Using ./bin/py.test
+ exec ./bin/py.test -x ./mediagoblin/tests --boxed
Traceback (most recent call last):
  File "$VENVS/mediagoblin/src/bin/py.test", line 33, in
<module>
    sys.exit(load_entry_point('pytest', 'console_scripts',
'py.test')())
  File "$VENVS/mediagoblin/src/bin/py.test",
line 22, in importlib_load_entry_point
    for entry_point in distribution(dist_name).entry_points
  File
"/usr/lib/python3.7/site-packages/importlib_metadata/__init__.py", line
558, in distribution return Distribution.from_name(distribution_name)
  File
"/usr/lib/python3.7/site-packages/importlib_metadata/__init__.py", line
215, in from_name
    raise PackageNotFoundError(name)
  importlib_metadata.PackageNotFoundError: No package metadata was
found for pytest

Funnily enough, I can execute ./bin/py.test ./mediagoblin/tests/ -x
once I switched into the repo. Without a bail (-x) I run into resource
exhaustion at about 42% of execution.


======================== test session starts =========================
platform linux -- Python 3.7.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: $VENVS/mediagoblin/src/mediagoblin/tests, configfile:
pytest.ini plugins: celery-4.2.2 collected 182 items / 3 skipped / 179
selected 
mediagoblin/tests/test_api.py ......................
mediagoblin/tests/test_audio.py F
============================== FAILURES ===============================
___________________________ test_transcoder ___________________________
Traceback (most recent call last):
  File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py", line
75, in test_transcoder with create_data_for_test() as (audio_name,
result_name): File "/usr/lib/python3.7/contextlib.py", line 112, in
__enter__ return next(self.gen)
  File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py", line
65, in create_data_for_test with create_audio() as audio_name:
  File "/usr/lib/python3.7/contextlib.py", line 112, in __enter__
    return next(self.gen)
  File "$VENVS/mediagoblin/src/mediagoblin/tests/test_audio.py", line
48, in create_audio
    pipeline.add(enc)
TypeError: Argument 1 does not allow None as a value
======= 1 failed, 22 passed, 3 skipped, 6 warnings in 53.78s ========

I took the liberty to add a new directory for testing JavaScript in the
browser using the Jasmine testing framework (MIT-licensed):
https://jasmine.github.io/api/3.10/global

This way, I can write a spec for the existing implementation before
rewriting it in more modern syntax. In order to run, you start a web
server (e.g. python3 -m http.server) and navigate to the
SpecRunner.html. I recommend to do so higher up in the directory tree
since I load the audio.js via directory traversal and the file has to
be found. The tests are then executed in the web browser.
Let me know if this is something you would be interested in adopting.
According to https://www.gnu.org/licenses/license-list.html#X11License
it is compatible to GPL.

> We may want to move the <script src="../audio.js"> to the end of the
> page body and drop all the ready() code.

I introduced some new blocks to collect them at the end (see the
base.html) called mediagoblin_scripts. It would be great if you could
already provide that upstream (even if empty).

For the sake of readability (and as experienced templater) I can
recommend to name the end-block tags:
https://jinja.palletsprojects.com/en/3.0.x/templates/#named-block-end-tags

I haven't grokked the plugin logic yet so added a `defer` attribute
there for now:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer
This requires testing.

> Feel free to consider the spirit of the code, rather than doing a
> literal translation.
> […]
> Feel free to translate any code to use ES6 features where they make
> things easier to read.

I wasn't exactly sure how far I can push this. `const` and `let` should
be safe enough by now: https://caniuse.com/const

ES6 class syntax is not supported in Internet Explorer and Opera Mini:
https://caniuse.com/es6-class
Would that pose a problem?

Some Node.js / Frontend-Tooling can understand
https://browserslist.dev/ if you are looking for a way to document the
supported browsers.

> I'd suggest adding a "use strict"; at the top.

Included (but not pushed yet). Thanks!

> Fade and other animations are absolutely not critical and you're
> welcome to just drop them if they're not trivial to implement.

I'm looking into it once I progressed on the tests. Ideally, I would
use CSS Animations similar to Svelte:


On Mon, 30 Aug 2021 12:00:16 +1000
Ben Sturmfels <ben@sturm.com.au> wrote:

> Hi Andre,
> 
> Woo! Great work on porting audio.js! Comments inline below.
> 
> On Sat, 28 Aug 2021, Andre Jaenisch wrote:
> 
> > Hey Ben,
> >
> > I've got started by replicating the repository on my private Gitea
> > instance. (I don't accept other accounts there, so don't look for a
> > sign-up page - in that sense, it is private).
> >
> > Next, I ran the tests (still five failing) and branched off to
> > tackle the first porting in 
> > https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/src/branch/refactor/port-audio-js
> >  
> 
> Can you send me details of the failing tests please and the commands
> you used to run them?
> 
> > The intermediate changes can be seen in:
> > https://jaenis.ch/hobbies/coding/repos/ryuno-ki/mediagoblin/compare/712728c331ea3518b1964bbc7fcf06ff2c567064...c00d3ad053d5d0862f1b71ec126e3c9e1493967d
> >  
> 
> Great stuff. A couple of comments:
> 
> We may want to move the <script src="../audio.js"> to the end of the
> page body and drop all the ready() code.
> 
> Feel free to consider the spirit of the code, rather than doing a
> literal translation. For example, the "spectrogram.addEventListener"
> code is doing some pretty complicated DOM traversal. I suspect this
> could be simplified with:
> 
>   var seekbar = document.querySelector('.audio-spectogram .seekbar');
>   seekbar.addEventListener(...
> 
> Fade and other animations are absolutely not critical and you're
> welcome to just drop them if they're not trivial to implement.
> 
> Feel free to translate any code to use ES6 features where they make
> things easier to read.
> 
> I'd suggest adding a "use strict"; at the top.
> 
> >
> > In order to continue on audio.js I need some assistance. The file
> > gets referenced in these places:
> >
> > -
> >   
> > extlib/sandyseventiesspeedboat/templates/mediagoblin/media_displays/audio.html
> > -
> >   
> > mediagoblin/plugins/archivalook/templates/archivalook/feature_displays/audio_head.html
> > - mediagoblin/templates/mediagoblin/media_displays/audio.html
> >
> > Which of them are relevant?
> > How can I navigate to them?  
> 
> > I want to make sure to not break something, but have to have a look
> > at the HTML to properly port the code (there are difference between
> > a single element being selected vs. multiple).
> >
> > My gut feeling would be to look at the templates one and ignore the
> > other.  
> 
> Yes, as you suggest, ignore the "sandyseventiesspeedboat" template,
> since this is external to the project (using git submodule) and no
> longer maintained. But change the other two. Archivalook is a plugin
> that you'll need to enable to test.
> 
> Regards,
> Ben