jayasurian123 / fen-validator

Validate the Chess FEN notation
10 stars 3 forks source link

Mistake in documentation #33

Closed pronode closed 9 months ago

pronode commented 1 year ago

It should be validateFen.default(...) not validateFen(...)

jayasurian123 commented 1 year ago

hi @pronode, could you tell me the scenario? Is it in the es6 syntax?

In the commonJs the documentation was written like this. const validateFEN = require('fen-validator').default;

pronode commented 1 year ago

Yes, in doc stands:

// ES6
import validateFEN from 'fen-validator';

validateFEN('rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR b KQkq - 0 1'); // true

should be:

// ES6
import validateFEN from 'fen-validator';

validateFEN.default('rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR b KQkq - 0 1'); // true

just tested with nodeJS

nlukic97 commented 9 months ago

This is correct. I have tested it as well, and these two options work with nodeJS:

commonjs:

const validateFEN = require('fen-validator').default;

validateFEN('rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR b KQkq - 0 1'); // true

ES6:

import validateFEN from 'fen-validator';

validateFEN.default('rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR b KQkq - 0 1'); // true

So the documentation should be updated, or the package should be updated - which would be a breaking change.

jayasurian123 commented 9 months ago

@nlukic97 I see whats the problem here. This package was written long back. That time there was no native node import. babel transpilers would work exactly as mentioned in the docs. The idea was commonjs is for node and es6 was for browsers (using babel).

Now the node is implemented import (as usual against all the standard), I would create a new major version which would support all the current techs including ts. 👍🏼

jayasurian123 commented 9 months ago

@nlukic97 Now the new major version is added. cjs, es, iife, umd supports are added. Now the TS types are also there. Now the default is no more needed. If possible please verify with the 2.0 version. 🙏🏼

nlukic97 commented 9 months ago

I've just tested it. It works. ✅

jayasurian123 commented 9 months ago

@nlukic97 Thank you :) I will close this issue then 👍🏼