Browser compatibility

[cesare-montresor]

'fs' is now optional, making it usable also by standard web browsers. is a nodeJS standard package, but is not usually available on browsers

[piranna]

Knowing what constants are needed, I would fully remove fs dependency and use directly the defined constants.

[cesare-montresor]

Indeed, I just made it this way so it leaves unchanged the behaviour any current setup, and adds a fallback for bowsers. But yes, honestly I'm not delighted by the try/catch on the require

[mafintosh]

Put it in a ./constants.js file and the make a browser field in package json that selects a compat one for browsers when bundling. You can't just use the inlined one always unfortunately as these are platform dependent

[piranna]

You can't just use the inlined one always unfortunately as these are platform dependent

But being platform dependent, they would bring incompatibilities between tar files created on different platforms, isn't it?

[cesare-montresor]

Ok, I refactored the solution a bit. By adding the "browser" entry inside package.json now the require('fs') now it fails "gracefully"

  "browser":{
    "fs": false
  }

The try/catch is replaced with a simple null/undefined check after the require('fs'), which eventually triggers the static constants fallback. This happens inside constants.js, which then exports the finalized constants. pack.js simply imports the constants for it.

[cesare-montresor]

@mafintosh After more testing I added a few more changes to the patch and now your library works inside a web browser (extract, for now). Most changes, other then "constants", are related to 'b4a' having a different interface between browser and nodejs versions. ( ex: .compare() )

@piranna do you use tar-stream on NodeJS on some of your projects? Would it be easy for you to check if this fork works just like the old one? https://github.com/cesare-montresor/tar-stream

I also open 2 issues describing the node/browser compatibility issues I found using b4a:

• https://github.com/holepunchto/b4a/issues/6
• https://github.com/holepunchto/b4a/issues/7

[piranna]

@piranna do you use tar-stream on NodeJS on some of your projects? Would it be easy for you to check if this fork works just like the old one?

It's been a long time I'm not using the package on my projects, my recomendation is that you do automated tests and include them in your PR.

[cesare-montresor]

@piranna do you use tar-stream on NodeJS on some of your projects? Would it be easy for you to check if this fork works just like the old one?

It's been a long time I'm not using the package on my projects, my recomendation is that you do automated tests and include them in your PR.

Are there examples of it in the repo for the nodejs parts ?

[piranna]

Are there examples of it in the repo for the nodejs parts ?

https://github.com/mafintosh/tar-stream/tree/master/test

[cesare-montresor]

Are there examples of it in the repo for the nodejs parts ?

https://github.com/mafintosh/tar-stream/tree/master/test

Ha, sorry, it makes sense. I looked inside my local node_modules/ folder only :(

[cesare-montresor]

Are there examples of it in the repo for the nodejs parts ?

https://github.com/mafintosh/tar-stream/tree/master/test

Ok, I managed to run successfully the build action, which also run the tests:

https://github.com/cesare-montresor/tar-stream/actions/runs/3939409919

[cesare-montresor]

As I need the lib patched to continue working on it ( otherwise everytime i do an npm install, it reverts ) I made it's own npm package, while remains a fork, so we can eventually merge it later, once is tested.

https://github.com/cesare-montresor/tar-web https://www.npmjs.com/package/tar-web