feat(linter/node): implement no-new-require #6165
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
CodSpeed Performance ReportMerging #6165 will not alter performanceComparing Summary
|
jelly
force-pushed
the
no-new-require
branch
from
b5e6360
to
f7b04d7
Compare
|
|
||
| let pass = vec
There was a problem hiding this comment.
Why should this be allowed?
There was a problem hiding this comment.
That's what the original rule allows as well, if we want to be more strict I would be happy to implement that behaviour.
| "var appHeader = require('app-header')", | ||
| "var AppHeader = new (require('app-header'))", | ||
| "var AppHeader = new (require('headers').appHeader)", | ||
| "var AppHeader = require('app-header'); var appHeader = new AppHeader();", |
There was a problem hiding this comment.
Can you please add a pass case for new require('foo')()
| fn no_new_require(span: Span) -> OxcDiagnostic { | ||
| OxcDiagnostic::warn("Unexpected use of new with require") | ||
| .with_label(span) | ||
| .with_help("Initialise the constructor separate from the import statement") |
There was a problem hiding this comment.
This message is a little confusing
There was a problem hiding this comment.
My idea was to inform the user to move the new call to a newline, any suggestions?
No description provided.