-
Notifications
You must be signed in to change notification settings - Fork 499
JavaScript: More key APIs are now non-async #6504
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* UUID generation fallback for Node version pre-14.17.0 * Apply suggestions from code review * Use module-load-time selection for performance * Polish --------- Co-authored-by: Tim te Beek <tim@moderne.io> Co-authored-by: Knut Wannheden <knut@moderne.io>
… a direct dependency if needed (#6434) * Allowing `AddDependency` to broaden the scope of existing dependency, provided you're requesting an equal or higher version number. If requesting same scope but higher version number, the version number will be upgraded. * Dropping `import` scope validity in favour of `compile`, given `import` is only applicable for dependencies in `dependencyManagement` Co-authored-by: Tim te Beek <tim@moderne.io> --------- Co-authored-by: Tim te Beek <tim@moderne.io>
* Drop Lombok hint from RemoveUnusedImports class Since we've supported Lombok for quite some time now. * Also update recipes.csv
* unit test with poc for UnfoldProperties * use imperative CopyValue in unit test * Invoke UnfoldProperties after MergeYaml * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Slight polish --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek <tim@moderne.io>
…en direct vs indirect dependency. (#6505) * If transitive, actually just allow regardless of version, as per old behaviour.
…PomDownloader` to prevent the situation where `/` was producing `/\pom.xml` on Windows rather than `\pom.xml` (#6506)
* Convert package-manager and related APIs from async to sync - Convert runInstallInTempDir and runWorkspaceInstallInTempDir to use sync fs APIs (fs.mkdtempSync, fs.writeFileSync, fs.readFileSync, fs.rmSync) - Convert runInstallIfNeeded callback from async to sync - Convert updateNodeResolutionMarker to sync (was marked async with no awaits) - Convert createLockFileEditor to use sync JsonVisitor and TreeVisitor - Update add-dependency, upgrade-dependency-version, and upgrade-transitive-dependency-version recipes to use sync visitors - Convert Result.diff() to sync (createTwoFilesPatch is already sync) The package manager operations were unnecessarily async since the underlying process spawning (spawnSync) was already synchronous. Only the file I/O was async, which is negligible compared to the npm/yarn/pnpm install time. * Remove unnecessary async from IsSourceFile.preVisit() * Add async property to TreeVisitor and AsyncTreeVisitor Add a readonly `async` property to both visitor base classes: - TreeVisitor: async = false - AsyncTreeVisitor: async = true This allows runtime discrimination of sync vs async visitors via the RecipeVisitor union type. Useful for sync-to-sync recipe composition where a sync visitor wants to call another sync visitor without async overhead. * Lift async I/O out of visitors into editorWithData() Refactor package-manager recipes to do async I/O (npm install) in editorWithData() before returning the visitor, keeping visitors pure. Changes: - Add async runInstallInTempDirAsync() using spawn() and fs.promises - Add async runWorkspaceInstallInTempDirAsync() for workspace support - Refactor AddDependency, UpgradeDependencyVersion, and UpgradeTransitiveDependencyVersion to run package manager installs in editorWithData() before returning visitors - Remove unused runInstallIfNeeded() helper function - Visitors are now pure tree transformations with no I/O This provides a cleaner separation of concerns: - Async I/O happens in the recipe's async editor() method - Visitors are pure, synchronous tree transformations using pre-computed data * More sync visitors * Simplify receivers by extending visitors --------- Co-authored-by: Claude <noreply@anthropic.com>
Clarified description for 'onlyIfUsing' option to specify its importance in multi-module projects. Fixes #5795
# Conflicts: # rewrite-core/src/main/java/org/openrewrite/rpc/RewriteRpc.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes sync visitors the default in the TypeScript OpenRewrite implementation, with async visitors available as an alternative. This change improves performance by up to 20x for visitor traversals that don't require async operations.
Key API Changes
Visitor Base Classes
TreeVisitorTreeVisitor(sync)AsyncTreeVisitorJavaVisitor<P>JavaVisitor<P>(sync)AsyncJavaVisitor<P>JavaScriptVisitor<P>JavaScriptVisitor<P>(sync)AsyncJavaScriptVisitor<P>TypeVisitor<P>TypeVisitor<P>(sync)AsyncTypeVisitor<P>JsonVisitor<P>JsonVisitor<P>(sync)AsyncJsonVisitor<P>YamlVisitor<P>YamlVisitor<P>(sync)AsyncYamlVisitor<P>PlainTextVisitor<P>PlainTextVisitor<P>(sync)AsyncPlainTextVisitor<P>ParseErrorVisitor<P>ParseErrorVisitor<P>(sync)AsyncParseErrorVisitor<P>TreePrinter API
TreePrinter.print(): Now returnsstringdirectly (sync)TreePrinters.register(): Takes syncTreeVisitor<any, PrintOutputCapture>callbackPattern & Template API
Pattern.match(): ReturnsMatchResult | undefined(sync)Pattern.matchWithExplanation(): ReturnsMatchAttemptResult(sync)Template.apply(): ReturnsJ | undefined(sync)PatternMatchingComparator.compare(): Returnsboolean(sync)Parser API
Parser.parseOne(): Changed fromPromise<SourceFile>toSourceFile(sync)parseOne():JsonParser,YamlParser,PlainTextParser,JavaScriptParserRPC Infrastructure
RpcCodec.rpcSend(): Changed fromPromisetovoidRpcSendQueuemethods: All converted to sync (no moreawaitneeded)JavaSender,TypeSender,JavaScriptSender,JsonSender,YamlSendernow extend sync visitor base classesMethod Signatures
Visitor methods changed from:
To:
Collection Mapping
mapSync()instead ofmapAsync()for synchronous visitor operationsmapAsync()still available for async visitorsMigration Guide
For code extending visitor base classes:
AsyncXxxVisitorinsteadAsyncXxxVisitoror proceed as in this exampleExample migration to new sync visitor base class: