-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Remove unused plus typescript tightening #3527
Remove unused plus typescript tightening #3527
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Some of these changes could make marked slower like declaring a variable inside a loop instead of before the loop. I'm not sure if this is still the case but it would be nice to get an idea of how this change affects speed.
In terms of speed it looks like this may be faster with Node v22. Running |
gives stricter TS typing, is more concise
I still have the bench-100 file from this PR #3146. It says main takes 122ms and this branch takes 117, which I think is kinda within margin of error. It's not a big degradation at least. |
cfe4305
to
3bc9c13
Compare
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.
Thanks! 💯
@@ -6,7 +6,7 @@ import type { Token, TokensList } from './Tokens.ts'; | |||
|
|||
export class _Hooks { | |||
options: MarkedOptions; | |||
block: boolean | undefined; | |||
block?: boolean; |
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.
Note that these do not have the same meaning.
https://x.com/DavidKPiano/status/1856701854102557026
Although I think in this case that's okay.
lastToken.raw += '\n' + token.raw; | ||
lastToken.text += '\n' + token.text; | ||
this.inlineQueue[this.inlineQueue.length - 1].src = lastToken.text; | ||
this.inlineQueue.at(-1)!.src = lastToken.text; |
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.
Not a fan of using !
but it is logically the same as the previous code.
We should look at making this safer in a follow up PR.
list.items[list.items.length - 1].raw = list.items[list.items.length - 1].raw.trimEnd(); | ||
list.items[list.items.length - 1].text = list.items[list.items.length - 1].text.trimEnd(); | ||
const lastItem = list.items.at(-1); | ||
if (lastItem) { |
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.
Thanks for making this one safe 👍
src/helpers.ts
Outdated
@@ -72,7 +72,7 @@ export function splitCells(tableRow: string, count?: number) { | |||
if (!cells[0].trim()) { | |||
cells.shift(); | |||
} | |||
if (cells.length > 0 && !cells[cells.length - 1].trim()) { | |||
if (!cells.at(-1)?.trim()) { |
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.
This looks like a bug.
Consider the case when cells = []
.
console.log(cells.length > 0 && !cells[cells.length - 1].trim()); // false
console.log(!cells.at(-1)?.trim()) // true
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.
Thanks for the catch. Since all the unit tests still pass, I wonder if for some reason cells.length always happens to be > 0 on this line. I'll fix it though.
## [15.0.1](v15.0.0...v15.0.1) (2024-11-18) ### Bug Fixes * Remove unused plus typescript tightening ([#3527](#3527)) ([1f579f7](1f579f7))
Description
This is a bunch of fairly minor changes to remove unused files/packages and take advantage of some more recent typescript/js features, namely
?:
in parameter definitions,?.
optional chaining, at.at(-x)
instead of[arr.length - x]
There should not be any changes in functionality or type interfaces.
It will make the most sense looked one commit at a time.
Contributor
Committer
In most cases, this should be a different person than the contributor.