Make usable from typescript file using npm imports #13
Conversation
Added types file. Included dependency on highlight.js to allow type specification to specify return type to avoid TypeScript linter errors. Added usage example.
|
Hey Keith, of course, always welcome! :) As you can see, there's not much activity in here, but I'll try to remind myself how to package and publish a new version to npm. ;) |
There was a problem hiding this comment.
I'm totally happy to have it merged as is, but I have some concerns about whether it would work the way you'd like it to. ;)
Regarding testing, probably https://github.com/highlightjs/highlight.js/blob/main/extra/3RD_PARTY_QUICK_START.md is the best source.
My main concern is that the files don't end up in tar.gz to be published to npm.
package.json
Outdated
| "devDependencies": { | ||
| "highlight.js": "^11.6.0" | ||
| } |
There was a problem hiding this comment.
It's a production dependency, not a dev one, isn't it?
Why this version?
There was a problem hiding this comment.
I'd suggest a looser > 11.x pin where x is maybe a more recent release... We're usually very serious about breaking changes. If it works with 11.0 it should work with 11.9, 11.24, etc...
There was a problem hiding this comment.
Changed to a dependency instead of dev dependency and set to ^11.x
@joshgoebel would you prefer I specifically make it > 11.x so that it will work with greater major versions too?
.types/tsql.d.ts
Outdated
There was a problem hiding this comment.
Worth following the convention from https://github.com/highlightjs/highlight.js/tree/main/types , so types rather than .types?
package.json
Outdated
| @@ -23,5 +24,8 @@ | |||
| "dist/tsql.min.js", | |||
| "dist/tsql.es.min.js", | |||
| "dist/ssms.min.css" | |||
There was a problem hiding this comment.
tsql.d.ts doesn't end up in the package when I run npm pack - should we list it files as per https://docs.npmjs.com/cli/v9/commands/npm-publish#files-included-in-package ?
Probably the same about src/languages/tsql.js now?
README.md
Outdated
| import hljs from 'highlight.js'; | ||
|
|
||
| import tsql from 'highlight.js-tsql'; | ||
| hljs.registerLanguage('tsql', tsql); |
There was a problem hiding this comment.
It doesn't work as it is in terms of importing the ssms.css, does it?
There was a problem hiding this comment.
I've updated the example to include the css import. Did a local pack and installed the package locally and the styles resolved successfully.
There was a problem hiding this comment.
Nice, thanks a lot!
LGTM - @joshgoebel, anything from you?
Hello, hopefully this Pull Request is welcome.
I was running into issues trying to use this in a project where I set up highlight.js from a typescript module. I was able to resolve this simply by adding a typing file and surfacing the src/languages/tsql.js file as the main import.
I don't think this will negatively affect CDN users, but I wasn't quite sure how to test that scenario, so help in that area would be appreciated.
I also incremented the minor version number, which seems appropriate, but I will the new version number up to you.
Please let me know if there is anything you'd like me to change.
Added types file. Included dependency on highlight.js to allow type specification to specify return type to avoid TypeScript linter errors. Added usage example.