fix: Migrate to Typescript #2805

DreierF · 2023-05-14T14:16:51Z

Description

This PR migrated the codebase to Typescript and automatically generates and publishes a declaration file which fixes #1732.
I tried to keep the actual code changes that change semantics of the code as small as possible. To simplify the publishing I replaced the rollup setup with tsup, which makes bundling the declaration files a lot easier. I started off with the declarations from Definitely Typed pulled them into the actual code as far as it made sense to me. I also copied over the tests from Definitely Typed.

In contrast to the Definitely Typed types the types are no longer part of the marked Namespace i.e. marked.MarkedOptions -> MarkedOptions as I did not find a way to preserve both the call signature on the namespace (marked(...)) in addition to exporting the types within the namespace. As this has no effect on runtime behavior and only requires trivial changes in Typescript code on user side this was the best tradeoff I could think of. Also namespaces are generally no longer recommended in Typescript so this seemed like a good direction anyway.

I also had to rename most internal symbols i.e. Tokenizer to _Tokenizer to fix an issue in the generated .d.ts file, which would otherwise look like

namespace marked {
   var Tokenizer: typeof Tokenizer; // Cyclic reference while the second Tokenizer actually did not mean the variable, but the import within the file. Also aliasing the classes on import did not fix the issue.
}

Future work

Obviously there is a bunch of places that could be more strictly/cleanly typed, but for now I focused on the exposed API as fixing the type errors within the functions will most of the time require code refactorings which would have made the PR un-reviewable.

@UziTech Please have a look and let me know what you think :)
I have pulled out the renaming from *.js to *.ts to a separate commit to make it easier to review.

Contributor

Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
no tests required for this PR.
If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

CI is green (no forced merge required).
Squash and Merge PR following conventional commit guidelines.

vercel · 2023-05-14T14:16:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
marked-website	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 29, 2023 5:14am

UziTech · 2023-05-15T19:47:30Z

Thank you for this! 💯

This looks like a great start. I have skimmed it, but I will do a more thorough review within the next week.

A couple things I see that could change:

try to remove any wherever possible. If it requires any add a comment about why and maybe what needs to change to update it in the future.
instead of @ts-ignore can we do @ts-expect-error with a comment why it errors. That will allow us to fix it easier in the future.

DreierF · 2023-05-16T16:27:27Z

Most of the any/ts-ignore were there because you'd have to change the actual code (not just the type annotations) to make it work with Typescript. I got rid of most any and ts-ignore now and replaced the remaining with ts-expect-error.

UziTech

This is going to be a breaking change so I think it will take some time to actually get merged in.

docs/demo/preview.html

src/Lexer.ts

src/Parser.ts

src/Renderer.ts

src/helpers.ts

DreierF · 2023-05-29T09:33:17Z

Thanks for the review. I hope addressed all your comments now and rebased the changes on master.

UziTech · 2023-06-10T03:14:09Z

@styfle @calculuschild should we focus on this for v6? and setting deprecated options to false?

calculuschild · 2023-06-10T03:33:28Z

Yeah, sounds good.

UziTech

@DreierF if you can rebase this I think we can merge this next so we don't have to keep rebasing.

DreierF · 2023-06-11T09:26:31Z

I'm done with the rebase. I squashed all the rework commits into a single commit to make the rebase less painful.

styfle · 2023-06-12T15:26:39Z

src/Instance.ts

+import { _Renderer } from './Renderer.js';
+import { _Tokenizer } from './Tokenizer.js';
+import { _TextRenderer } from './TextRenderer.js';
+import { _Slugger } from './Slugger.js';


Why are these prefixed with underscores?

Is there precedence for this convention?

As I mentioned already in the description, I had to rename most internal symbols i.e. Tokenizer to _Tokenizer to fix an issue in the generated .d.ts file, which would otherwise look like

namespace marked { var Tokenizer: typeof Tokenizer; // Cyclic reference while the second Tokenizer actually did not mean the variable, but the import within the file. Also aliasing the classes on import did not fix the issue. }

Do you have any better idea on how to overcome this issue?
You should be able to revert that once you get rid of the marked export and it's extension fields.

I don't have a solution, but I'll second that I don't like the underscores.

UziTech

This looks like a good start. We can fix the anys and ignores in future PRs.

tsconfig-type-test.json

tsconfig.json

UziTech · 2023-07-29T02:52:35Z

@calculuschild do you have any other concerns? I would like to get this merged soon.

calculuschild · 2023-07-29T04:54:16Z

I guess we can merge. I feel like there are probably some more tweaks we can make to clear up the typing syntax in a few places but I think that can be pushed to a later PR.

UziTech · 2023-07-29T04:58:04Z

Ya I think we should merge this and get a couple other breaking changes in before releasing v6.

@DreierF if you can rebase this I can merge it.

Co-authored-by: Steven <steven@ceriously.com>

DreierF · 2023-07-29T05:15:39Z

Great to hear! Rebase is done ✅

# [6.0.0](v5.1.2...v6.0.0) (2023-07-31) ### Bug Fixes * Migrate to Typescript ([#2805](#2805)) ([cb54906](cb54906)) ### BREAKING CHANGES * Migrate to Typescript

vercel bot deployed to Preview May 14, 2023 14:17 View deployment

DreierF force-pushed the typescript branch from 7b15b77 to 72d4bdb Compare May 15, 2023 18:43

vercel bot deployed to Preview May 15, 2023 18:43 View deployment

DreierF force-pushed the typescript branch from 72d4bdb to fa39ce6 Compare May 15, 2023 19:02

vercel bot deployed to Preview May 15, 2023 19:03 View deployment

vercel bot deployed to Preview May 16, 2023 16:24 View deployment

DreierF force-pushed the typescript branch from 603e8bb to 56b6ecb Compare May 16, 2023 16:24

vercel bot deployed to Preview May 16, 2023 16:25 View deployment

UziTech requested changes May 24, 2023

View reviewed changes

DreierF force-pushed the typescript branch from 56b6ecb to be8d083 Compare May 29, 2023 09:22

vercel bot deployed to Preview May 29, 2023 09:22 View deployment

DreierF force-pushed the typescript branch from be8d083 to e671358 Compare May 29, 2023 09:24

vercel bot deployed to Preview May 29, 2023 09:25 View deployment

vercel bot deployed to Preview May 29, 2023 09:31 View deployment

DreierF mentioned this pull request May 29, 2023

fix: Replace custom repeatString function with repeat() #2820

Merged

5 tasks

DreierF requested a review from UziTech June 8, 2023 11:24

This was referenced Jun 8, 2023

TypeScript: Deceleration or Migrate markedjs/marked-xhtml#37

Closed

TypeScript: Deceleration or Migrate markedjs/marked-mangle#37

Closed

UziTech requested changes Jun 10, 2023

View reviewed changes

DreierF force-pushed the typescript branch from 6163260 to 9131f86 Compare June 11, 2023 09:23

vercel bot deployed to Preview June 11, 2023 09:23 View deployment

DreierF requested a review from UziTech June 11, 2023 09:26

styfle reviewed Jun 12, 2023

View reviewed changes

calculuschild mentioned this pull request Jul 3, 2023

Pass token to renderer methods ? #2640

Closed

DreierF force-pushed the typescript branch from be881db to 5501d92 Compare July 8, 2023 11:35

vercel bot deployed to Preview July 8, 2023 11:35 View deployment

vercel bot deployed to Preview July 9, 2023 06:28 View deployment

UziTech approved these changes Jul 19, 2023

View reviewed changes

styfle reviewed Jul 25, 2023

View reviewed changes

tsconfig-type-test.json Outdated Show resolved Hide resolved

styfle reviewed Jul 25, 2023

View reviewed changes

tsconfig.json Outdated Show resolved Hide resolved

vercel bot deployed to Preview July 25, 2023 14:04 View deployment

styfle approved these changes Jul 25, 2023

View reviewed changes

calculuschild approved these changes Jul 29, 2023

View reviewed changes

DreierF and others added 9 commits July 29, 2023 07:06

Renamed src/**.js to src/**.ts

7eb567a

Converted code to Typescript

4330a0f

Rework

0b27157

Rework

a6aab3d

Moved Rule and Rules types to rules.ts

ccc116a

Extract reflink type

3d777aa

Extracted Links type to src/Tokens

8d1bbb6

Downgrade target to es2021

626b738

Co-authored-by: Steven <steven@ceriously.com>

Updated bundled files

aae4d6e

DreierF force-pushed the typescript branch from 65504a7 to aae4d6e Compare July 29, 2023 05:14

vercel bot deployed to Preview July 29, 2023 05:14 View deployment

UziTech changed the title ~~Migrate to Typescript~~ fix: Migrate to Typescript Jul 29, 2023

UziTech merged commit cb54906 into markedjs:master Jul 29, 2023
10 checks passed

DreierF deleted the typescript branch July 29, 2023 06:45

github-actions bot pushed a commit that referenced this pull request Jul 31, 2023

chore(release): 6.0.0 [skip ci]

9927247

# [6.0.0](v5.1.2...v6.0.0) (2023-07-31) ### Bug Fixes * Migrate to Typescript ([#2805](#2805)) ([cb54906](cb54906)) ### BREAKING CHANGES * Migrate to Typescript

NatoBoram mentioned this pull request Nov 13, 2023

Migrate from @ts-stack/markdown to marked NatoBoram/Leanish#93

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Migrate to Typescript #2805

fix: Migrate to Typescript #2805

DreierF commented May 14, 2023 •

edited

Loading

vercel bot commented May 14, 2023 •

edited

Loading

UziTech commented May 15, 2023

DreierF commented May 16, 2023

UziTech left a comment

DreierF commented May 29, 2023

UziTech commented Jun 10, 2023

calculuschild commented Jun 10, 2023

UziTech left a comment

DreierF commented Jun 11, 2023

styfle Jun 12, 2023

DreierF Jun 24, 2023

calculuschild Jun 27, 2023

UziTech left a comment

UziTech commented Jul 29, 2023

calculuschild commented Jul 29, 2023

UziTech commented Jul 29, 2023

DreierF commented Jul 29, 2023

fix: Migrate to Typescript #2805

fix: Migrate to Typescript #2805

Conversation

DreierF commented May 14, 2023 • edited Loading

Description

Future work

Contributor

Committer

vercel bot commented May 14, 2023 • edited Loading

UziTech commented May 15, 2023

DreierF commented May 16, 2023

UziTech left a comment

Choose a reason for hiding this comment

DreierF commented May 29, 2023

UziTech commented Jun 10, 2023

calculuschild commented Jun 10, 2023

UziTech left a comment

Choose a reason for hiding this comment

DreierF commented Jun 11, 2023

styfle Jun 12, 2023

Choose a reason for hiding this comment

DreierF Jun 24, 2023

Choose a reason for hiding this comment

calculuschild Jun 27, 2023

Choose a reason for hiding this comment

UziTech left a comment

Choose a reason for hiding this comment

UziTech commented Jul 29, 2023

calculuschild commented Jul 29, 2023

UziTech commented Jul 29, 2023

DreierF commented Jul 29, 2023

DreierF commented May 14, 2023 •

edited

Loading

vercel bot commented May 14, 2023 •

edited

Loading