Expose the "lib" folder to consumers

[binyamin]

Scenario

The current package.json limits access to a single file, via a conditional export. This entry-point is the only file that consumers may import.

music-metadata/package.json

Lines 14 to 19 in 72a4006

	"exports": {
	".": {
	"node": "./lib/index.js",
	"default": "./lib/core.js"
	}
	},

Personally, I was trying to import ./lib/ParserFactory.js, so I could check whether a given format is supported.

Solutions

1. Use subpath patterns to export the entire lib folder. Compatible with Node.js v14.13.0 & v12.20.0, and greater. I think that with v16.x you can specify extensions. I testing this method locally with v14.20.0

{
  "exports": {
    ".": { 
      "node": "./lib/index.js", 
      "default": "./lib/core.js" 
    },
    "./lib/*": "./lib/*",
    // you can restrict access to subfolders
    "./lib/internal/*": null
  }
}

2. Keep using a single entry-point, and expose various functions, classes, etc., via that file
3. Add an "exports" field for specific files, instead of exposing the entire folder.

[Borewit]

With the introduction of ESM you cannot just import from some sub-path. Basically you propose to re-enable that, but I think there are good reasons to keep the implementation private. Splitting everything is a internal (private) part and in a not so private part, I am not so sure that is the way to go.

If I would change any of my internally functionality I would break API compatibility.
I think it is better to turn it the other way around.
What functionality do you need, which is currently not exposed?

And let's expose that in a clean and documented way.

Do you need something like?:

function isMimeTypeSupported(mime)

function isFileSupported(path)

Can you create a PR?

[binyamin]

@Borewit The "exports" field appears to be a Node.js thing, not an "ESM" thing. I'm using Deno, which only uses ESM, and that's not required at all.

[binyamin]

But that's a good idea. I can work on a PR.

[binyamin]

@Borewit I'm a bit conflicted on this. It may be smarter for users to catch the error when a file cannot be parsed. This would require a custom error class for the ParserFactory, which would be useful to have in general.

Otherwise, here's the basic code I would add. I assume it would go in lib/core.ts, and lib/index.ts would re-export it.

/**
 * Determines whether the {@link filepath} is supported for parsing.
 * @param filepath Path, filename or extension of a file.
 */
export function isFileSupported(filepath: string) {
    return Boolean(ParserFactory.getParserIdForExtension(filepath));
}

[Borewit]

It is already close to do something like that:

music-metadata/lib/ParserFactory.ts

Line 100 in 7cdebd1

throw new Error('Failed to determine audio format');

&

music-metadata/lib/ParserFactory.ts

Line 105 in 7cdebd1

throw new Error('Guessed MIME-type not supported: ' + guessedType.mime);

If we would throw an error with a good described error code, would that provide you the feedback you are looking for?

What is the objective?

[binyamin]

the object is to throw a subclass of Error, instead of a generic Error class, so I can use instanceof to identify the code that threw the error.

class ParserError extends Error { /* ... */ }

async function parse() {
    throw new ParserError("...");
}

try {
    await parse();
} catch (error) {
    if (error instanceof ParserError) {
        // This must be from music-metadata
    } else {
        throw error;
    }
}

[binyamin]

@Borewit What do you think about using a custom error class? As it stands, I still cannot tell which errors are coming from this library in particular.

Also, I just want to reiterate that the "exports" field is a Node.js creation, meant as an alternative to the "main" field. It is not related to ESM, and is not required.

[Borewit]

Sorry for this very late reply, custom error class sounds good to me.