fix: npm and JSR package contents #16
Conversation
| const contents = fs.readFileSync(filePath, "utf8"); | ||
|
|
||
| // prepend the reference comment | ||
| const newContents = `/// <reference types="./${filename}.d.ts" />\n${contents}`; |
There was a problem hiding this comment.
Can we remove the contents of types.ts here? This will make the package smaller
There was a problem hiding this comment.
No. As I said in the PR description, JSR errors without types.ts. It needs to stay.
There was a problem hiding this comment.
I meant to delete its contents and keep only the triple-slash directive. not delete this file
packages/compat/package.json
Outdated
| "dist/cjs/index.cjs", | ||
| "dist/cjs/index.d.cts", | ||
| "dist/cjs/types.ts", | ||
| "dist/cjs/types.d.ts", | ||
| "dist/esm/index.js", | ||
| "dist/esm/index.d.ts", | ||
| "dist/esm/types.ts", | ||
| "dist/esm/types.d.ts", | ||
| "README.md", | ||
| "LICENSE" |
There was a problem hiding this comment.
It looks like we could just include "dist" here instead of each single file. BTW, README and LICENSE are included automatically regardless of extension, so it's not required to list them explicitly: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#files.
There was a problem hiding this comment.
Ah good call. I was copy-pasting between jsr.json and package.json.
packages/config-array/package.json
Outdated
| "dist/cjs/index.cjs", | ||
| "dist/cjs/index.d.cts", | ||
| "dist/cjs/types.ts", | ||
| "dist/cjs/types.d.ts", | ||
| "dist/esm/index.js", | ||
| "dist/esm/index.d.ts", | ||
| "dist/esm/types.ts", | ||
| "dist/esm/types.d.ts", | ||
| "README.md", | ||
| "LICENSE" |
packages/object-schema/package.json
Outdated
| "dist/cjs/index.cjs", | ||
| "dist/cjs/index.d.cts", | ||
| "dist/cjs/types.ts", | ||
| "dist/cjs/types.d.ts", | ||
| "dist/esm/index.js", | ||
| "dist/esm/index.d.ts", | ||
| "dist/esm/types.ts", | ||
| "dist/esm/types.d.ts", | ||
| "README.md", | ||
| "LICENSE" |
Co-authored-by: Francesco Trotta <github@fasttime.org>
Co-authored-by: Francesco Trotta <github@fasttime.org>
|
Updated the |
| "dist/esm/types.d.ts", | ||
| "README.md", | ||
| "LICENSE" | ||
| "dist" |
There was a problem hiding this comment.
types.ts again, we don't need it in npm.
|
@kecrily I'm leaving @fasttime @mdjermanovic ready for re-review. |
|
I'm not worried about its current state, just that it might balloon in the future and the core still needs to be rewritten. Not sure how big core's |
When I tried publishing to JSR, I got several errors. Specifically:
types.tsfile to be present because it is referenced fromindex.js. Because this is a TypeScript error, I also addedtypes.tsto the npm package..d.tsfiles and so couldn't find the type definitions for the packages, it needs a triple-slash directive. The triple-slash directive must be added aftertscis run, otherwise it looks for that file to verify the contents. So I had to add a new tool calledprepend-type-ref.jsthat runs aftertscin each package.I verified locally via
npx jsr publish --dry-runthat the JSR publish will succeed with these changes.