Skip to content

Commit 10619b8

Browse files
All @openrewrite/rewrite modules must only be loaded once (#6131)
The complexity exists because we're working around a fundamental limitation of Node.js's module system. Here's why each part is necessary: What the code does: 1. Iterates all cached modules - No way around this; we need to find what's already loaded 2. Converts filesystem paths → module specifiers - This is the "complex" part, but it's unavoidable 3. Maps cache entries - Simple once we have the paths The path conversion complexity is unavoidable because: - require.cache keys: `/absolute/path/node_modules/@openrewrite/rewrite/dist/tree.js` - Module imports: `require('@openrewrite/rewrite/tree')` - We must reverse-engineer one from the other
1 parent b6cf748 commit 10619b8

3 files changed

Lines changed: 128 additions & 23 deletions

File tree

rewrite-javascript/rewrite/src/javascript/preconditions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ export function hasSourcePath(filePattern: string): Promise<RpcRecipe> | TreeVis
2727
}
2828

2929
export function usesMethod(methodPattern: string, matchOverrides: boolean = false): Promise<RpcRecipe> | TreeVisitor<any, ExecutionContext> {
30-
return RewriteRpc.get() ? RewriteRpc.get()!.prepareRecipe("org.openrewrite.java.search.UsesMethod", {
30+
return RewriteRpc.get() ? RewriteRpc.get()!.prepareRecipe("org.openrewrite.java.search.FindMethods", {
3131
methodPattern,
3232
matchOverrides
3333
}) : new UsesMethod(methodPattern);
3434
}
3535

3636
export function usesType(fullyQualifiedType: string): Promise<RpcRecipe> | TreeVisitor<any, ExecutionContext> {
37-
return RewriteRpc.get() ? RewriteRpc.get()!.prepareRecipe("org.openrewrite.java.search.UsesType", {
37+
return RewriteRpc.get() ? RewriteRpc.get()!.prepareRecipe("org.openrewrite.java.search.FindTypes", {
3838
fullyQualifiedType,
39-
includeImplicit: false
39+
checkAssignability: false
4040
}) : new UsesType(fullyQualifiedType);
4141
}

rewrite-javascript/rewrite/src/rpc/request/install-recipes.ts

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -142,28 +142,39 @@ export class InstallRecipes {
142142
}
143143
}
144144

145+
/**
146+
* Ensures dynamically loaded modules share the same class instances as the host
147+
* by mapping require.cache entries. This prevents instanceof failures when the
148+
* same package is installed in multiple node_modules directories.
149+
*/
145150
function setupSharedDependencies(targetModulePath: string) {
146-
const sharedDeps = [
147-
'@openrewrite/rewrite',
148-
'vscode-jsonrpc',
149-
];
150-
151-
sharedDeps.forEach(dep => {
152-
try {
153-
// Get your already-loaded version
154-
const yourDepPath = require.resolve(dep);
155-
const yourModule = require.cache[yourDepPath];
156-
157-
if (yourModule) {
158-
// Find where the target would look for this dependency
159-
const targetDir = path.dirname(targetModulePath);
160-
const targetDepPath = require.resolve(dep, {paths: [targetDir]});
161-
162-
// Make the target use your cached version
163-
require.cache[targetDepPath] = yourModule;
151+
const sharedDeps = ['@openrewrite/rewrite', 'vscode-jsonrpc'];
152+
const targetDir = path.dirname(targetModulePath);
153+
154+
sharedDeps.forEach(depName => {
155+
const depPattern = path.sep + 'node_modules' + path.sep + depName.replace('/', path.sep);
156+
157+
for (const cachedPath of Object.keys(require.cache)) {
158+
if (!cachedPath.includes(depPattern)) continue;
159+
160+
try {
161+
// Extract subpath: /path/node_modules/@pkg/dist/tree.js -> dist/tree.js
162+
const pkgIndex = cachedPath.indexOf(depPattern);
163+
let subpath = cachedPath.substring(pkgIndex + depPattern.length)
164+
.replace(/^[/\\]/, '') // Remove leading slash
165+
.replace(/\.(js|ts)$/, '') // Remove extension
166+
.replace(/^dist[/\\]/, '') // Remove dist/ prefix if present
167+
.replace(/[/\\]index$/, ''); // Remove /index suffix
168+
169+
// Build require path: @pkg or @pkg/subpath
170+
const requirePath = subpath ? `${depName}/${subpath}` : depName;
171+
172+
// Resolve from target's perspective and map cache
173+
const targetDepPath = require.resolve(requirePath, {paths: [targetDir]});
174+
require.cache[targetDepPath] = require.cache[cachedPath];
175+
} catch (e) {
176+
// Target can't resolve this path, skip
164177
}
165-
} catch (e) {
166-
// Module not found or not resolvable, skip
167178
}
168179
});
169180
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
import * as path from 'path';
17+
18+
describe('Shared dependencies setup', () => {
19+
let originalCache: any;
20+
21+
beforeEach(() => {
22+
// Save original require.cache
23+
originalCache = {...require.cache};
24+
});
25+
26+
afterEach(() => {
27+
// Restore original require.cache
28+
Object.keys(require.cache).forEach(key => {
29+
if (!originalCache[key]) {
30+
delete require.cache[key];
31+
}
32+
});
33+
});
34+
35+
it('should map all subpaths of shared dependencies', () => {
36+
// This test verifies the logic of mapping multiple cached modules
37+
// In a real scenario, dynamically loaded recipes would benefit from this
38+
39+
// Simulate what happens when modules are cached
40+
const mockCache: Record<string, any> = {
41+
'/some/path/node_modules/@openrewrite/rewrite/tree.js': {exports: {}},
42+
'/some/path/node_modules/@openrewrite/rewrite/recipe.js': {exports: {}},
43+
'/some/path/node_modules/@openrewrite/rewrite/visitor.js': {exports: {}},
44+
};
45+
46+
// Count modules from our mock that match the pattern
47+
const depPattern = path.sep + 'node_modules' + path.sep + '@openrewrite/rewrite'.replace('/', path.sep);
48+
const rewriteModules = Object.keys(mockCache).filter(p => p.includes(depPattern));
49+
50+
// Should find all three mocked modules
51+
expect(rewriteModules.length).toBe(3);
52+
53+
// Verify different subpaths exist
54+
const hasTreeModule = rewriteModules.some(p => p.includes(path.sep + 'tree'));
55+
const hasRecipeModule = rewriteModules.some(p => p.includes(path.sep + 'recipe'));
56+
const hasVisitorModule = rewriteModules.some(p => p.includes(path.sep + 'visitor'));
57+
58+
expect(hasTreeModule).toBe(true);
59+
expect(hasRecipeModule).toBe(true);
60+
expect(hasVisitorModule).toBe(true);
61+
});
62+
63+
it('should handle package names with slashes correctly', () => {
64+
const depPattern = path.sep + 'node_modules' + path.sep + '@openrewrite/rewrite'.replace('/', path.sep);
65+
66+
// Pattern should work correctly with scoped packages
67+
expect(depPattern).toContain('@openrewrite' + path.sep + 'rewrite');
68+
});
69+
70+
it('should extract subpaths correctly', () => {
71+
const testCases = [
72+
{
73+
path: '/path/to/node_modules/@openrewrite/rewrite/tree.js',
74+
expected: '/tree.js'
75+
},
76+
{
77+
path: '/path/to/node_modules/@openrewrite/rewrite/index.js',
78+
expected: '/index.js'
79+
},
80+
{
81+
path: '/path/to/node_modules/@openrewrite/rewrite/rpc/recipe.js',
82+
expected: '/rpc/recipe.js'
83+
}
84+
];
85+
86+
testCases.forEach(tc => {
87+
const depPattern = path.sep + 'node_modules' + path.sep + '@openrewrite/rewrite'.replace('/', path.sep);
88+
const packageIndex = tc.path.indexOf(depPattern);
89+
const afterPackage = tc.path.substring(packageIndex + depPattern.length);
90+
91+
expect(afterPackage).toBe(tc.expected);
92+
});
93+
});
94+
});

0 commit comments

Comments
 (0)