-
Notifications
You must be signed in to change notification settings - Fork 1
feat: javascript code parser for transformations #337
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
base: chore.transformation-spec
Are you sure you want to change the base?
feat: javascript code parser for transformations #337
Conversation
🔒 Scanned for secrets using gitleaks 8.28.0
abhimanyubabbar
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.
Some clarifying questions
| func (p *JavaScriptParser) ValidateSyntax(code string) error { | ||
| result := api.Transform(code, api.TransformOptions{ | ||
| Loader: api.LoaderJS, | ||
| }) |
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.
Isn't there an option regarding validating against a specific ECMAScript ?
I mean what we are supporting, we will only validate against those core JS conditionals and types right ?
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 v0 transformation follow commonjs format. we need to parse both commonjs and ecmascript
| for _, err := range result.Errors { | ||
| errorMsgs = append(errorMsgs, err.Text) | ||
| } | ||
| return fmt.Errorf("javascript syntax error: %s", strings.Join(errorMsgs, "; ")) |
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.
| return fmt.Errorf("javascript syntax error: %s", strings.Join(errorMsgs, "; ")) | |
| return fmt.Errorf("javascript syntax error: \n\t%s", strings.Join(errorMsgs, "\n\t")) |
This might give better readability, wdyt ?
| // Transform the code to extract dependencies | ||
| // Using Transform with JSX loader to handle modern JS syntax | ||
| result := api.Transform(code, api.TransformOptions{ | ||
| Loader: api.LoaderJSX, |
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.
What's the difference between the JSX loader vs JS loader used above ?
|
|
||
| // extractImportsFromTransformedCode extracts import paths from esbuild-transformed code | ||
| // esbuild preserves import/require statements in the output, making them easy to find | ||
| func extractImportsFromTransformedCode(code string) map[string]bool { |
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 would just name this function as
extractImports(code string) map[string]bool
| // Scan through the code looking for import/require statements | ||
| // esbuild's output is well-formatted, making this reliable |
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.
Do we support both statements ( import and require ) in js code in transformation ?
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.
we do not support require. filtering them
| } | ||
|
|
||
| // extractQuotedString extracts the first quoted string from text | ||
| func extractQuotedString(text string) string { |
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.
It seems like a very complex function, can you explain the values which this function can receive and can we simplify it's complexity ?
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.
It does miss detecting first and last quotes as same ?
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 used claude to try and simplify the function implementation for a bit more readable and I think it generated the output in accordance the expectations:
package parser
import (
"regexp"
"strings"
)
type JavaScriptParser struct{}
// ExtractImports extracts all non-relative library imports from JavaScript code
func (p *JavaScriptParser) ExtractImports(code string) []string {
// Step 1: Remove comments to avoid false matches
code = removeComments(code)
// Step 2: Extract all import statements
imports := extractAllImports(code)
// Step 3: Filter out relative/absolute paths
imports = filterNonRelativeImports(imports)
// Step 4: Deduplicate
return deduplicateImports(imports)
}
with each implementation downstream ...
🔒 Scanned for secrets using gitleaks 8.28.0
🔒 Scanned for secrets using gitleaks 8.28.0
…e-parser 🔒 Scanned for secrets using gitleaks 8.28.0
🔒 Scanned for secrets using gitleaks 8.28.0
resolves DAW-2752