nfroidure / ttf2woff2

Convert ttf files to woff2.
MIT License
297 stars 39 forks source link

package.json module field points to non existent file #81

Closed shannon closed 1 month ago

shannon commented 1 year ago

Issue

I'm a gentledev I:

Expected behavior

When importing this module as ESM using tools that rely on the module field in the package.json such as esm.sh, skypack or rollup, I expect the module to be loaded correctly as ESM.

Actual behavior

The module field points to src/index.mjs which does not exist in the repo or in the published npm files. Because of this, tools such as esm.sh try to load the file in the module field assuming it is ESM compatible and fail.

Also, the dist/index.mjs that does get compiled to is not actually an ESM compatible file as it is identical to the CJS file dist/index.js. So I don't think it's just a matter of switching it to dist/index.mjs. The issue appears to be both that the build process does not correctly output ESM and the module field is pointing to the wrong place.

Steps to reproduce the behavior

I discovered this issue while trying to run the fantasticon package in a Deno environment.

import { generateFonts } from 'https://esm.sh/fantasticon';

await generateFonts();
console.log('Done', results);
deno run -A build.js

I observed an Internal 500 error from esm.sh for file https://esm.sh/v104/ttf2woff2@4.0.5/deno/ttf2woff2.js

/* esm.sh - error */
throw new Error("[esm.sh] " + "resovleESModule: open /tmp/esm-build-0f4e49f989dd168d1181c711f2d6f99ec5230a77-3eaf098a/node_modules/ttf2woff2/src/index.mjs: no such file or directory");
export default null;

You can find a similar error at skypack https://cdn.skypack.dev/ttf2woff2

/*
 * [Package Error] "ttf2woff2@v5.0.0" could not be built. 
 *
 *   [1/5] Verifying package is valid…
 *   [2/5] Installing dependencies from npm…
 *   [3/5] Building package using esinstall…
 *   Running esinstall...
 *   We resolved "ttf2woff2" to ttf2woff2/src/index.mjs, but the file does not exist on disk.
 *   We resolved "ttf2woff2" to /tmp/cdn/_AZLc81T2K6TQbp4BYyb6/node_modules/ttf2woff2/src/index.mjs, but the file does not exist on disk.
 *
 * How to fix:
 *   - If you believe this to be an error in Skypack, file an issue here: https://github.com/skypackjs/skypack-cdn/issues
 *   - If you believe this to be an issue in the package, share this URL with the package authors to help them debug & fix.
 *   - Use https://skypack.dev/ to find a web-friendly alternative to find another package.
 */

console.warn("[Package Error] \"ttf2woff2@v5.0.0\" could not be built. \n[1/5] Verifying package is valid…\n[2/5] Installing dependencies from npm…\n[3/5] Building package using esinstall…\nRunning esinstall...\nWe resolved \"ttf2woff2\" to ttf2woff2/src/index.mjs, but the file does not exist on disk.\nWe resolved \"ttf2woff2\" to /tmp/cdn/_AZLc81T2K6TQbp4BYyb6/node_modules/ttf2woff2/src/index.mjs, but the file does not exist on disk.");
throw new Error("[Package Error] \"ttf2woff2@v5.0.0\" could not be built. ");
export default null;

Furthermore, if you try to run the dist/index.mjs file with node you will get an error as it is not ESM compatible.

 node node_modules/ttf2woff2/dist/index.mjs
file:///<redacted>/node_modules/ttf2woff2/dist/index.mjs:6
  module.exports = require('../jssrc/index.js');
  ^

ReferenceError: module is not defined in ES module scope
    at file:///<redacted>/node_modules/ttf2woff2/dist/index.mjs:6:3
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5)

If ESM is not really a supported use case for this project, then I would recommend just removing the module field all together (and dist/index.mjs) as these tools can typically convert CJS, they are just getting confused by its presence. I can't imagine this would be a breaking change because neither are currently functional.

I can submit a PR to either remove both or correct the build process if you want.

nfroidure commented 1 month ago

Switched to ESM so closing