aadsm / jsmediatags

Media Tags Reader (ID3, MP4, FLAC)
Other
748 stars 128 forks source link

Fix invalid ArrayBuffer -> Number cast #79

Closed chrisparton1991 closed 6 years ago

chrisparton1991 commented 6 years ago

Fixes #77.

I ran npm test before my changes and got 7 failing tests. I'm not sure if it's because I'm running on Windows?

I also ran Flow after my change and got a warning, but I've never used Flow and don't know how to fix that one.

aadsm commented 6 years ago

Can you paste the failures you got?

chrisparton1991 commented 6 years ago

Sure thing, here are the errors: flow-failures.txt test-failures.txt

I'm running Windows 10.0.15063, Node 8.4.0 and NPM 5.4.2.

aadsm commented 6 years ago

@chrisparton1991, I’ve fixed both tests and flow and pushed them to master so it should be fine now, thanks for warning (I should really setup travis). There’s only one thing missing now, can you add a test that catches the bug you fix to avoid having regressions in the future?

chrisparton1991 commented 6 years ago

Thanks for getting that stuff fixed up! I can run all of the tests without issue now.

The unit tests already appear to be catching the issue. With my fix they all pass, without the fix I get:

 FAIL  src\__tests__\BlobFileReader-test.js
  ● BlobFileReader › should read a byte

    Offset 0 hasn't been loaded yet.
MartinDawson commented 6 years ago

@aadsm Can this be merged please? Thanks for fixing @chrisparton1991.

chrisparton1991 commented 6 years ago

No problem @MartinDawson, glad I could help :)

All glory to the Hypnotoad!

MartinDawson commented 6 years ago

hehe, very helpful indeed as it was causi... ALL GLORY TO THE HYPNOTOAD!

aadsm commented 6 years ago

Awesome, thank you so much for fixing this!

chrisparton1991 commented 6 years ago

No problem, thanks for making an awesome library :)

I'm using jsmediatags in my LED sequencer project (https://github.com/chrisparton1991/sparkled), it's been super useful.

aadsm commented 6 years ago

good to hear! that project looks pretty cool :)