streamich / memfs

JavaScript file system utilities
http://streamich.github.io/memfs/
Apache License 2.0
1.72k stars 131 forks source link

use somewhat newer build target? #791

Closed jimmywarting closed 1 year ago

jimmywarting commented 2 years ago

Your target is set to es5 and your lib references ES2017

https://github.com/streamich/memfs/blob/dec6bda7cd3b374277ccb11a0bdb38be69124bae/tsconfig.json#L3-L4

think it generates a lot of unnecessary and old boilercode... would you consider bumping it up to somewhat newer target version?

G-Rath commented 2 years ago

When I last mentioned it to @streamich he was keen to continue supporting Node 4+but that was a few years ago so he might be open to a breaking change.

streamich commented 2 years ago

What do you think about having multiple build targets? For example, there could be two build targets, say es5 in /lib folder and es2020 in /es2020 folder. Something like this.

Idea being that /lib could be used in old browser and es2020/ could be used in latest Node.js version.

G-Rath commented 2 years ago

@streamich I don't think there's a whole lot of advantage to that vs the con of having a larger package size and extra complexity (since you now have to think about where you import from, which also is something IDEs don't understand).

Having multiple build targets like that are typically only useful for browsers if you're not using a build chain, but it's very common these days to be using something like webpack for tree shaking etc which means it's very easy to have it also transpile memfs if folks really want to support older browsers. We've also now got CDNs like skypack which can let folks continue to avoid build tools by doing the building & transpiling for you.

That's not to say that we should target the absolute latest build target - I think we should at least target ES6 (because it's now actually pretty old and supported by all current browsers, and even has partial support on IE11), but not fussed if we don't move the target any higher for now (My preference would be around Node 10/ES2018, because pre-10 is a bit of a mess and would allow native async/await & generators which are the biggest cost to transpile in terms of extra code).

Something to keep in mind is that transpiling is syntax only, and most of the costly & big ticket syntaxes were added in the Node 8-10 band (namely generators and async/await) - so if we move our sole build target to be that area it would remove most of our transpile overhead; this also means that people only older systems would need to polyfill anyway which tends to have a higher overhead than transpiling dependencies 🤷

jimmywarting commented 2 years ago

perhaps take this into consideration? by allowing memfs to support IE you will basically giving folks a reason not to switch to a newer and a more secure browser. it circle back to all other developers who gets frustrated that folks don't update their browser or the NodeJS version.

it would also have been nice if you could output esm so it would be as easy as importing memfs from a cdn

williamstein commented 1 year ago

I'm using the es2020 target for https://www.npmjs.com/package/@cowasm/memfs

https://github.com/sagemathinc/memfs-js/commit/5b51afa5dd85c6e0c1f387ee174c9b974516c404

devinea commented 1 year ago

What is the current thinking on ES target? All NodeJS versions still in maintenance support newer ES versions. Also IE is no longer supported.

I am starting to see issues using memfs using recent versions of TypeScript I think it is related to https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#more-compliant-indirect-calls-for-imported-functions

Since target is es5 the lib code includes this which seems to be undefined when used in TS4.4+.

G-Rath commented 1 year ago

@devinea not much has changed - while a higher build target would be nice, I don't think it would be worth the overhead of a new major when this library has been pretty stable.

I've been using memfs with latest TypeScript without issue, and we're actually using 4.7+ for the project - if you open a new issue with some more details about the issues you're seeing, we can have a look into it.

devinea commented 1 year ago

Thanks @G-Rath FYI: It seems changing in my project from

import { vol } from 'memfs';

to

import * as memfs from 'memfs';
const vol = memfs.vol;

works as fine with TypeScript 4.4+.

streamich commented 1 year ago

The v4 will have es6 build target, it is currently tagged as next on NPM, try it out, see next branch.