Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smoother API Surfaces #47

Open
Offroaders123 opened this issue Aug 25, 2024 · 3 comments
Open

Smoother API Surfaces #47

Offroaders123 opened this issue Aug 25, 2024 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed

Comments

@Offroaders123
Copy link
Owner

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 Offroaders123 added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed labels Aug 25, 2024
@Offroaders123 Offroaders123 self-assigned this Aug 25, 2024
Offroaders123 added a commit that referenced this issue Aug 25, 2024
Looking into restructuring the layout of the public API, both in how you import from it, and how the exported functionalities are named. When I check out how people are using the library, it seems like they don't read the documentation too closely (understandable), so they use extra API calls that don't have to be there. It seems like people want to use `NBT.parse()` more than `NBT.read()`, while one is for binary files, and one is for SNBT files. I can see how they might think `read()` is for reading from the file system, and maybe instead they think `parse()` is to just read arbitrary data. NBTify is meant to be used specifically outside of the file system though, so neither API feature would worry about that. Not all libraries are like that though, so I can see why people think that.

#47

This is a demo I made the other day of how it might make sense to see the public API look. This commit experiments with that as a demo in mind.

```js
// @ts-check

import { Buffer } from "node:buffer";
import * as NBT from "nbtify";

const buffer = Buffer.alloc(25);
const data = await NBT.readBinary(buffer);

NBT.writeBinary; NBT.readString; NBT.writeString;
```
@Offroaders123
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant