fix: Better support for .vue. and .svelte files using jscodeshift-adapters
#71
Conversation
There was a problem hiding this comment.
Hey @fnimick thanks for opening this PR and thanks for going through the trouble of publishing this library! I had a quick look at the source code and think the idea of using the framework-specific tools to parse and extract scripts is generally good!
To be transparent, we're a bit hesitant about adding new dependencies to our tools, so before merging this PR in, I'll bring it up with the team to ensure we're on the same page.
Also, just to confirm since the diffs look a bit wild: the general pattern for the changes is:
- remove
wrapJscodeshiftcalls - replace with
adaptcalls - remove any resulting unnecessary variables (like
fileName)
The rest is just auto-formatting, right?
|
Also, just as a heads-up: I'm assigning myself to this issue to track internally that I'm reviewing it. No worries though, I'm not gonna take over or anything. Happy to let you continue if we need to make further changes. |
|
@Lms24 yep, that's all I did. There was one other incredibly minor change to make typescript happy (adding an optional chain) but I will flag that for you specifically. If you want an easier diff, turn 'ignore whitespace' on, which will hide all the lines where the only changes was the indentation from removing the edit: I also added names to most of the formerly anonymous transform functions, matching their directory names, so they show up in stack traces for easier debugging (and so I could |
| @@ -30,7 +32,7 @@ module.exports = function transform(fileInfo, api, options) { | |||
|
|
|||
| /** { @type import('jscodeshift').ASTPath<import('jscodeshift').ImportDeclaration> | undefined */ | |||
| const sentryNamespaceImport = sentryNamespaceImports.length ? sentryNamespaceImports.get() : undefined; | |||
| const sentryNameSpace = sentryNamespaceImport?.node.specifiers?.[0].local?.name; | |||
There was a problem hiding this comment.
@Lms24 this is the only other code change, adding ? to [0]?.local because typescript was unhappy without it.
There was a problem hiding this comment.
Man I had no idea about "Hide Whitespaces" thanks a lot for the hint!
As for this change, I'm gonna make the call and say adding the dependency is okay. Given the change is well isolated in this PR, we could (not saying we will) easily revert if we received bug reports.
Thanks for contributing, I'll make sure you're mentioned in the changelog :)
Lms24
changed the title
.vue. and .svelte files using jscodeshift-adapters.vue. and .svelte files using jscodeshift-adapters
Resolves #70