Offroaders123 / NBTify

A library to read and write NBT files on the web!
http://npm.im/nbtify
MIT License
42 stars 5 forks source link

Smoother API Surfaces #47

Open Offroaders123 opened 2 months ago

Offroaders123 commented 2 months ago

What makes for a nice API to use? I want NBTify to be usable in a plain ESM context (accessible by CDN from the browser), so import specifiers can't be too complex, things like that.

My main concern is that it seems like when I find people using NBTify in the wild, they mix up how features are meant to be used, and I think it's because the names I chose for the APIs aren't as explanatory as they could be. I am looking into how this could be improved, and how to make it more obvious as to how NBTify accomplishes it's design goals, without having to explain it outright to new users; it should just click!

Offroaders123 commented 2 months ago

Another part of the confusion might be my current documentation around the module exports too.

Originally, I liked the concept of modeling NBTify's module usage to be similar to that of the built-in JSON object, but that might be unnecessary. I think feature-wise it does make sense to still model around how that works, but naming-wise, maybe it should be more specific to the things that NBTify accomplishes.

// Namespace take

import * as NBT from "nbtify";

declare const buffer: Uint8Array;

const data: NBT.NBTData = await NBT.read(buffer);
const recompile: Uint8Array = await NBT.write(data);

const stringed: string = NBT.stringify(data);
// This would be `NBT.NBTData` as well, except that SNBT doesn't have any
// binary format options, so it wouldn't make sense to return those I think.
const destringed: NBT.RootTag = NBT.parse(stringed);
// Named import take

import { read, write, parse, stringify, type NBTData, type RootTag } from "nbtify";

declare const buffer: Uint8Array;

const data: NBTData = await read(buffer);
const recompile: Uint8Array = await write(data);

const stringed: string = stringify(data);
const destringed: RootTag = parse(stringed);

So the first initial step would be to look into renaming these exports, and making the namespace import just a choice, rather than a recommendation. The names should stand out enough on their own to make sense as a named import alone, without needing context of the namespace.

// Updated named imports

import { readBinary, writeBinary, readString, writeString, type NBTData, type RootTag } from "nbtify";

declare const buffer: Uint8Array;

const data: NBTData = await readBinary(buffer);
const recompile: Uint8Array = await writeBinary(data);

const stringed: string = writeString(data);
const destringed: RootTag = readString(stringed);

I think this can help distinguish which parts of the API are meant for NBT, and which are for SNBT. I thought this would be more apparent, but I think I was using it more as a gimmick or something.

Offroaders123 commented 2 months ago

The other parts around things like NBTData seemed to have had trouble in the past too, it's essentially my way of handling being able to effortlessly pass function results around, without needing to destructure things, or access results by properties.

In other libraries, some seem to return a tuple with the data object, and the other with the format options object. I do like this approach, but it means you have to destructure a specific part of the result in order to access the value, and passing it back and forth between readBinary() and writeBinary() wouldn't seem as straightfoward to me, because I think you should be able to easily call writeBinary(readBinary(bufferValue)), without needing to destructure anything, or manually keep reference to the format that it returns. I like this as a concept, also from JSON.stringify(JSON.parse('{"hi-mom": "😃"}')) in concept.

I guess NBTData doesn't necessarily have to be a class itself, nor an object which holds a data property, and it could instead just be a tuple that writeBinary() would have to accept as a parameter, just like it does now for either a RootTag | NBTData object. So, I think I hadn't thought about that one before. It does seem like that could be kind of smart. I'm also curious if you could leverage that to keep track of a single format object too, since the array would reference the same object you passed into readBinary(). That seems kind of hectic too though, I wouldn't want to rely on object references to persist something across multiple files. I guess that could work, but seems a bit iffy. It's the argument for/against mutability or immutable duplication.

Offroaders123 commented 2 months ago

Another example demo I started conceptualizing with, this is the kind of thing I intend and personally use NBTify like.

import { readFile } from "node:fs/promises";
import { readBinary, readString } from "nbtify";

const buffer = await readFile("./hello_world.nbt");
const nbt = await readBinary(buffer);

const string = await readFile("./bigtest.snbt", "utf-8");
const snbt = await readString(string);