-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix: Publish types (+ validate and use types + generate type maps) #21
Conversation
package.json
Outdated
}, | ||
"files": [ | ||
"index.js", | ||
"index.d.ts", | ||
"index.d.ts.map", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should publish a Source Map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? It's just a way to ensure that "go to source" in eg. VSCode actually still goes to the source and not to the type file?
And its only this big:
{"version":3,"file":"index.d.ts","sourceRoot":"","sources":["index.js"],"names":[],"mappings":"AA+BA,0CAHW;IAAE,KAAK,CAAC,EAAE,OAAO,GAAG,SAAS,CAAA;CAAE,GAC7B,OAAO,CAgFnB"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related: microsoft/TypeScript#14479
And https://www.typescriptlang.org/docs/handbook/project-references.html#declarationmaps
But sure, I can remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed ✔️
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: LitoMore <LitoMore@users.noreply.github.com>
Anything I can do to help push this further? |
package.json
Outdated
"test": "xo && nyc ava", | ||
"create-types": "tsc index.js --allowJs --declaration --emitDeclarationOnly" | ||
"test": "xo && nyc ava && tsc", | ||
"create-types": "tsc -p declaration.tsconfig.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the long-flag rather than -p
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
index.js
Outdated
function parseVersion(versionString) { | ||
if (/^\d{3,4}$/.test(versionString)) { | ||
// Env var doesn't always use dots. example: 4601 => 46.1.0 | ||
const m = /(\d{1,2})(\d{2})/.exec(versionString); | ||
const m = /(\d{1,2})(\d{2})/.exec(versionString) ?? []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to either upgrade XO or not use ??
. I suggest not using ??
as that's a less invasive change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed @sindresorhus
Not sure why master branch is failing when this PR passed. https://github.com/jamestalmage/supports-hyperlinks/actions/runs/4363207180 |
I noticed that this package was generating types but then told
npm
to only publishbrowser.js
andindex.js
and not the actually generated types.This fixes that + generates declarations maps + validates the types with TS
strict=true
.It also adds a
tsconfig.json
to tell eg. VSCode that hey, this repository wants you to make use of TS types inindex.js
.Tell me if you want me to pull something of this PR and I'll make it more granular 👍
Note: I bumped the lowest supported Node-version to a reasonable current LTS-focused version range so that the
tsconfig.json
file could be pointed towards a reasonable fitting definition with correctlib
definition:es2020
Fixes #15
Closes #20