skpm / fs

Drop-in replacement for the fs NodeJS package
MIT License
27 stars 11 forks source link

Use ES2015 exports #6

Closed appsforartists closed 6 years ago

appsforartists commented 6 years ago

Would you accept a PR that replaced the CommonJS exports with ES2015 ones? e.g.

module.exports.readFileSync = //...
// becomes
export function readFileSync(//...

The motivation would be to make this library consumable from a broader variety of build tools.

mathieudutour commented 6 years ago

The issue with using export function is that it's not possible to import fs from '@skpm/fs' anymore.

What build tools are you talking about? I don't expect people to actually use this directly, it will be shipped in Sketch.

appsforartists commented 6 years ago

Sure you can:

import * as fs from '@skpm/fs';

We use Bazel with TypeScript and Closure Compiler. I don't believe it resolves CommonJS, but we can do ES2015 imports.

When will this ship in Sketch? I thought these packages had to be imported by plugin authors.

mathieudutour commented 6 years ago

import fs from '@skpm/fs' !== import * as fs from '@skpm/fs'

As it should be a drop-in replacement, it should work with import fs from '@skpm/fs' (as fs does).

I mean CommonJS is pretty much a standard, tons of npm packages are using just that. A build tool that doesn't support it is... weird...

Anyhow, I need CommonJS for the require algorithm implemented in Sketch. I do not have a timeline as to when it will be shipped tho.

appsforartists commented 6 years ago

I'm a bit confused. You want it to work with an incorrect ES2015 syntax, even though it doesn't use ES2015 now?

Can whatever build tool you're using cross compile import * as fs from 'fs' to require('fs')?

appsforartists commented 6 years ago

Another option, if you really don't like *

export default {
  readFileSync,
  writeFileSync,
  //etc.
}

would give you import fs from 'fs'.

mathieudutour commented 6 years ago

I don't want to use any build system. Sketch 49 brings a new global: require. So you will be able to require('fs') without build system. To make it work, i need this repo to be in CommonJS.

appsforartists commented 6 years ago

Ahhh, if I can use it in Sketch without changing my build system, that works too. Thanks!