Open qgustavor opened 5 years ago
The build step uses the typescript compiler API rather than tsc
directly to add JSDoc annotations to the output (@constructor
, @enum
, @memberof
, etc) and for generating the docs, both of which require type information.
It also modifies the compiler output using UJS with some custom steps to strip multiple definitions of the compiler helpers like __extends
and to minify better by renaming private members (UJS does not rename them by default because JS doesn't have a concept of private members).
But both of these custom steps are only for quality-of-life improvements. Just running tsc
should produce a usable file that has the same API as the one built with the build script, and just minifying it with any minifier at default settings will produce a usable minified file.
The build step uses the typescript compiler API [...]
So I need to update the build step to use the new compiler API in order to make it work with the new Typescript version.
Just running tsc should produce a usable file that has the same API as the one built with the build script [...]
I don't know a lot about Typescript, but I tried running npm install --save-dev typescript@latest; npx tsc --outfile lib\libjass.js src\index.ts
and it returned those errors and no file. Am I missing something?
Edit: I tried npx tsc -p src\tsconfig.json
and it's now returning results. As I'm compiling it for Node I removed dom lib and I need to remove parts of code that depend on it.
Edit 2: I got it to compile without warnings. Instead of upgrading I just removed those quality-of-life improvements and features that I'm not planning to use (like rendering and web worker support). You can close this issue if you prefer.
So I need to update the build step to use the new compiler API in order to make it work with the new Typescript version.
To clarify what I said, the current script hooks into the compiler internals because "add JSDoc comments to the compiler output" is not something the compiler API supports. So it's almost certain it won't work without modification if you update the compiler, which is why you got the error that you got in the OP.
I tried running
npm install --save-dev typescript@latest; npx tsc --outfile lib\libjass.js src\index.ts
and it returned those errors and no file. Am I missing something?
As you found, you need to give it the path of the project file (tsconfig.json
), not the index.ts
As I'm compiling it for Node I removed dom lib and I need to remove parts of code that depend on it.
Did you do this because you got compiler errors with the new TS's dom.d.ts
? Or just to slim down the output?
Edit 2: I got it to compile without warnings. Instead of upgrading
Right, I wasn't suggesting that you actually try to update the build script code to work with newer TS. That's why I said to just run tsc
directly.
I didn't really have a plan to update the TS version because the current one is good enough for a project that isn't being developed any more, ie it works and produces a usable output. Newer TS obviously has more typing features but they aren't particularly necessary for a library that only gets shipped as JS.
But if the current TS version stops working at some point because node.js itself breaks it (say by removing the deprecated Buffer
constructor), I suppose there is value in getting rid of the custom build stuff.
Did you do this because you got compiler errors with the new TS's dom.d.ts ? Or just to slim down the output?
I'm making a fork focused on subtitle parsing, modification and serialization. I used to work with ass-parser, but it's a bit feature lacking compared with libjass. I'm sure I would mess things if I tried to implement tag parsing myself - because I already messed it - so I decided to make a fork.
I wasn't suggesting that you actually try to update
If I updated it I could send a pull request fixing this issue. I needed to remove docs, if I fix the build script I can get the docs working again and I can help fixing this issue.
In terms of priorities, the work is:
Update TS to current version, even if at the cost of not having custom build steps for docs or synthesized JSDoc comments.
Be able to build docs. IIRC this relies entirely on public TS compiler API, so it might still need to be updated but it won't be using any hacks.
Be able to synthesize JSDoc comments. As I said before, currently this uses hacks to be able to access the compiler internals. But it seems TS had added a compiler API for it - https://github.com/microsoft/TypeScript/blame/master/lib/typescript.d.ts#L4110 - so hopefully the hacks aren't needed any more.
If you're up to doing (2) and even (3), that would be amazing! But you said you aren't familiar with TS, and I imagine even less familiar with the TS compiler API. So even if you send a PR for just (1) that'll be great.
I'm trying to fix everything: started with a fresh clone, updated Typescript to latest version and tried fixing errors or commenting parts of the code until it compiles. By now it's compiling, but Field ... has no @type annotation.
, There are @param annotations ...
and Unbound type parameter ...
are silenced (probably because TS API changed) and docs aren't building. If possible I will send a pull request soon.
Edit: I will not send a pull request because my code isn't working. Those are the changes I did at the moment: https://github.com/qgustavor/libjass/commit/218a0952efd87129fde3471d1573f737c2965c38
I thought that if I just looked change logs and commits on Typescript repository and used those to update the code to the newer Typescript API it would solve the issue, but seems it's more complicated than I expected. It's using not only the public Typescript API but also the private API too. Maybe is better rewriting the JSDoc adding part of code... I will pause working on it, I need to rewrite parts of my code that were using ass-parser to use libjass now.
The current Typescript version is returning this warning:
DeprecationWarning: Buffer() is deprecated due to security and usability issues.
Trying to update to a newer version breaks the code:
Why Typescript is being extended by compiler.ts? Is possible to rewrite build.js without depending on those changes?