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
chore: upgrade dlint and run prefer-primordials
#11777
Conversation
"exclude": [ | ||
"no-invalid-triple-slash-reference" | ||
] |
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.
If I understand correctly, no-invalid-triple-slash-reference
rule is for Deno users, not for the internal code of Deno, so I added it here. Please correct me if I'm wrong.
https://lint.deno.land/#no-invalid-triple-slash-reference
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.
That's correct
// deno-lint-ignore prefer-primordials | ||
Error.prepareStackTrace = prepareStackTrace; |
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.
In Node.js, Error.prepareStackTrace
is skipped linting because of the config. But deno_lint doesn't support it right now, so just added an ignore directive here for the moment.
async function dlintPreferPrimordials() { | ||
const execPath = getPrebuiltToolPath("dlint"); | ||
console.log("prefer-primordials"); | ||
|
||
const sourceFiles = await getSources(ROOT_PATH, [ | ||
"runtime/**/*.js", | ||
"ext/**/*.js", | ||
"core/**/*.js", | ||
":!:core/examples/**", | ||
]); | ||
|
||
if (!sourceFiles.length) { | ||
return; | ||
} | ||
|
||
const chunks = splitToChunks(sourceFiles, `${execPath} run`.length); | ||
for (const chunk of chunks) { | ||
const p = Deno.run({ | ||
cmd: [execPath, "run", "--rule", "prefer-primordials", ...chunk], | ||
}); | ||
const { success } = await p.status(); | ||
if (!success) { | ||
throw new Error("prefer-primordials failed"); | ||
} | ||
p.close(); | ||
} | ||
} |
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 think prefer-primordials
should apply only to the code related to bootstrapping, so I created another function where only this rule is run.
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.
Makes sense, please add a comment explaining that
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.
Done 👍 0cf3bfb
d4f6888
to
0cf3bfb
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.
This is awesome! Well done @magurotuna.
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.
LGTM, thanks @magurotuna this is very useful
This PR upgrades
third_party
submodule to the latest to get dlint 0.12.0, and appliesprefer-primordials
to the files under runtime, ext, and core.