-
Notifications
You must be signed in to change notification settings - Fork 62
W-20161235: Add getter komaci rules to offline analysis tool #326
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
W-20161235: Add getter komaci rules to offline analysis tool #326
Conversation
|
|
||
| const baseName = code.name; | ||
|
|
||
| bundleAnalyzer.setLwcBundleFromContent(baseName, jsCode, htmlCode); |
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.
Load both JS and HTM files into bundle. Sometimes getter in JS is referenced from 'html' files.
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.
All HTML templates have been passed in setLwcBundleFromContent
| } | ||
|
|
||
| private analyzeIssues(code: string, messages: Linter.LintMessage[]): ExpertCodeAnalysisIssuesType { | ||
| private analyzeIssues(code: string, messages: Linter.LintMessage[], jsPath: string): ExpertCodeAnalysisIssuesType { |
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.
Add the file path to help the model quickly locate the file containing the issues.
| export const LwcCodeSchema = z.object({ | ||
| name: z.string().min(1).describe('Name of the LWC component'), | ||
| namespace: z.string().describe('Namespace of the LWC component').default('c'), | ||
| js: LwcFileSchema.describe('LWC component JavaScript file.'), |
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.
LWC has only 1 JS file
packages/mcp-provider-mobile-web/src/tools/offline-analysis/get_mobile_lwc_offline_analysis.ts
Outdated
Show resolved
Hide resolved
packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts
Outdated
Show resolved
Hide resolved
packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts
Outdated
Show resolved
Hide resolved
khawkins
left a comment
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.
The only change I'd like to see is not artificially limiting the number of LWC templates in the analysis. Otherwise, this passes muster to me.
packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts
Outdated
Show resolved
Hide resolved
khawkins
left a comment
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.
Just a formatting issue suggestion in the prompt, but otherwise this looks fine to me.
…leConfig.ts Co-authored-by: Kevin Hawkins <khawkins@salesforce.com>
| "dedent": "^1.5.3", | ||
| "@salesforce/mcp-provider-api": "^0.4.0", | ||
| "@salesforce/eslint-plugin-lwc-graph-analyzer": "^1.0.0", | ||
| "@salesforce/eslint-plugin-lwc-graph-analyzer": "^1.1.0-beta.1", |
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.
Has a non-prerelease version of this been released?
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.
No, this beta includes the Komaci 260 release, so the official graph analyzer release will need to happen in the 262 timeframe.
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.
@iowillhoit A bit of backstory on this one. eslint-plugin-lwc-graph-analyzer depends on the Komaci static analyzer, and that in turn has a dependency on core major releases, where changes can be breaking. Now in this case:
- Other than the Komaci dependency update, the code line of
eslint-plugin-lwc-graph-analyzerhas otherwise been stable for quite some time. - The Komaci change itself is mostly a bug fix that we wanted for this PR, and otherwise as a product isn't changing significantly anymore.
- Given that the Komaci change is technically a part of the next major release, we were nevertheless reluctant to make the dependency a part of the
latestpublication, just to follow that cadence. So we're effectively staging the change, with no other anticipated updates, until the next major release lands. - But given what we know about the actualities of the change, it felt okay to consume that change ourselves, here, since all of the code is stable and more or less static these days.
Anyway, I wanted to give you the details that went into the decision. Happy to discuss more, if you have questions.
…rcecli#326) * feat: add getter komaci rules to offline analysis tool of mobile-web-mcp * test: add tests for getter komaci rules for offline analysis tool of `mobile-web-mcp` server * fix: change based on feedback * fix: change based on feedback. Make instructions more AI friendly * Update packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts Co-authored-by: Kevin Hawkins <khawkins@salesforce.com> * fix: update graph-analyzer with node 20 engine --------- Co-authored-by: Kevin Hawkins <khawkins@salesforce.com>
…rcecli#326) * feat: add getter komaci rules to offline analysis tool of mobile-web-mcp * test: add tests for getter komaci rules for offline analysis tool of `mobile-web-mcp` server * fix: change based on feedback * fix: change based on feedback. Make instructions more AI friendly * Update packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts Co-authored-by: Kevin Hawkins <khawkins@salesforce.com> * fix: update graph-analyzer with node 20 engine --------- Co-authored-by: Kevin Hawkins <khawkins@salesforce.com>
…rcecli#326) * feat: add getter komaci rules to offline analysis tool of mobile-web-mcp * test: add tests for getter komaci rules for offline analysis tool of `mobile-web-mcp` server * fix: change based on feedback * fix: change based on feedback. Make instructions more AI friendly * Update packages/mcp-provider-mobile-web/src/tools/offline-analysis/ruleConfig.ts Co-authored-by: Kevin Hawkins <khawkins@salesforce.com> * fix: update graph-analyzer with node 20 engine --------- Co-authored-by: Kevin Hawkins <khawkins@salesforce.com>
feat: Add getter komaci rules to offline analysis tool
What does this PR do?
This PR adds getter violation detection features and comprehensive tests to the mobile-web-mcp server's offline analysis tool. The implementation includes Komaci rules for validating getter usage in Lightning Web Components (LWC) to ensure mobile-ready patterns.
What issues does this PR fix or reference?
Gus Item: @W-20161235@
Changes
feat: add getter komaci rules to offline analysis tool of mobile-web-mcp
test: add tests for getter komaci rules for offline analysis tool of
mobile-web-mcpserverFiles Changed
Summary
Impact
This change improves the mobile-web-mcp server's ability to detect and report getter-related violations in Lightning Web Components, helping developers write more mobile-friendly code.