Skip to content
This repository was archived by the owner on Jun 30, 2026. It is now read-only.

Commit 866a1d2

Browse files
CopilotjeremymengJialinHuang803Copilot
authored
Correct #platform subpath mappings in generated package metadata and emitted helper imports (#3997)
* Initial plan * fix platform subpath imports in generated package metadata and helper imports Agent-Logs-Url: https://github.com/Azure/autorest.typescript/sessions/2d603179-9c3b-465f-a2a0-60f5faa2f36d Co-authored-by: jeremymeng <7583839+jeremymeng@users.noreply.github.com> * fix: gate react-native platform import on generateReactNativeTarget in updatePackageFile The fresh-generation path in getEsmEntrypointInformation (packageCommon.ts) correctly gates the react-native condition on the generateReactNativeTarget option (PR #3943), but updatePackageFile in buildPackageFile.ts was always re-injecting react-native into the #platform/* imports map on regeneration. Mirror the fresh-generation behavior: only include the react-native condition when generateReactNativeTarget is true, and preserve key ordering (browser, react-native, default) for Node's conditional resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeremymeng <7583839+jeremymeng@users.noreply.github.com> Co-authored-by: JialinHuang803 <jialinhuang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: JialinHuang803 <139532647+JialinHuang803@users.noreply.github.com>
1 parent d219163 commit 866a1d2

9 files changed

Lines changed: 202 additions & 20 deletions

File tree

packages/rlc-common/src/metadata/buildPackageFile.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,30 @@ export function updatePackageFile(
144144
return;
145145
}
146146

147-
// Ensure warp packages have #platform/* imports for polyfill resolution
147+
// Ensure warp packages have #platform/* imports for polyfill resolution.
148+
// The `react-native` condition is only added when explicitly opted in via
149+
// `generateReactNativeTarget`, matching the fresh-generation path in
150+
// `getEsmEntrypointInformation` (packageCommon.ts).
148151
if (needsPlatformImportsUpdate) {
149-
packageInfo.imports = {
150-
"#platform/*.js": {
151-
browser: "./src/*-browser.mjs",
152-
"react-native": "./src/*-react-native.mjs",
153-
default: "./src/*.js"
154-
}
152+
const platformImports: Record<string, string> = {
153+
browser: "./src/*-browser.mts",
154+
default: "./src/*.ts"
155155
};
156+
if (model.options?.generateReactNativeTarget) {
157+
// Insert `react-native` before `default` so Node's conditional
158+
// resolution order matches the fresh-generation output.
159+
packageInfo.imports = {
160+
"#platform/*": {
161+
browser: platformImports.browser,
162+
"react-native": "./src/*-react-native.mts",
163+
default: platformImports.default
164+
}
165+
};
166+
} else {
167+
packageInfo.imports = {
168+
"#platform/*": platformImports
169+
};
170+
}
156171
}
157172

158173
// Update exports based on build system (warp for monorepo, tshy for others)

packages/rlc-common/src/metadata/packageJson/packageCommon.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ function getEsmEntrypointInformation(config: PackageCommonInfoConfig) {
118118
types: "./dist/commonjs/index.d.ts",
119119
browser: "./dist/browser/index.js",
120120
imports: {
121-
"#platform/*.js": {
122-
browser: "./src/*-browser.mjs",
123-
default: "./src/*.js"
121+
"#platform/*": {
122+
browser: "./src/*-browser.mts",
123+
default: "./src/*.ts"
124124
} as Record<string, string>
125125
},
126126
exports: resolveWarpExports(
@@ -131,15 +131,15 @@ function getEsmEntrypointInformation(config: PackageCommonInfoConfig) {
131131

132132
if (config.generateReactNativeTarget) {
133133
result["react-native"] = "./dist/react-native/index.js";
134-
(result.imports["#platform/*.js"] as Record<string, string>)[
134+
(result.imports["#platform/*"] as Record<string, string>)[
135135
"react-native"
136-
] = "./src/*-react-native.mjs";
136+
] = "./src/*-react-native.mts";
137137
// Reorder so react-native comes before default
138-
const importsEntry = result.imports["#platform/*.js"] as Record<
138+
const importsEntry = result.imports["#platform/*"] as Record<
139139
string,
140140
string
141141
>;
142-
result.imports["#platform/*.js"] = {
142+
result.imports["#platform/*"] = {
143143
browser: importsEntry.browser,
144144
"react-native": importsEntry["react-native"],
145145
default: importsEntry.default

packages/rlc-common/test/integration/packageJson.spec.ts

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,13 @@ describe("Package file generation", () => {
278278
// Default: no react-native entrypoint
279279
expect(packageFile).to.not.have.property("react-native");
280280
expect(packageFile).to.have.property("exports");
281+
expect(packageFile).to.have.property("imports");
282+
expect(packageFile.imports).to.deep.equal({
283+
"#platform/*": {
284+
browser: "./src/*-browser.mts",
285+
default: "./src/*.ts"
286+
}
287+
});
281288
expect(packageFile.exports["./package.json"]).to.equal("./package.json");
282289
expect(packageFile.exports["."]).to.have.property("browser");
283290
// Default: no react-native in exports
@@ -304,6 +311,14 @@ describe("Package file generation", () => {
304311
"react-native",
305312
"./dist/react-native/index.js"
306313
);
314+
expect(packageFile).to.have.property("imports");
315+
expect(packageFile.imports).to.deep.equal({
316+
"#platform/*": {
317+
browser: "./src/*-browser.mts",
318+
"react-native": "./src/*-react-native.mts",
319+
default: "./src/*.ts"
320+
}
321+
});
307322
expect(packageFile.exports["."]).to.have.property("react-native");
308323
expect(packageFile.exports["."]["react-native"]).to.deep.equal({
309324
types: "./dist/react-native/index.d.ts",
@@ -1087,9 +1102,83 @@ describe("Package file generation", () => {
10871102
"^2.3.1"
10881103
);
10891104

1090-
// Platform imports should be added for Azure monorepo ESM packages
1105+
// Platform imports should be added for Azure monorepo ESM packages.
1106+
// By default (generateReactNativeTarget=false) the `react-native`
1107+
// condition must NOT be emitted, matching the fresh-generation path.
1108+
expect(packageFile).to.have.property("imports");
1109+
expect(packageFile.imports).to.deep.equal({
1110+
"#platform/*": {
1111+
browser: "./src/*-browser.mts",
1112+
default: "./src/*.ts"
1113+
}
1114+
});
1115+
});
1116+
1117+
it("should include react-native in platform imports when generateReactNativeTarget is true", () => {
1118+
const model = createMockModel({
1119+
moduleKind: "esm",
1120+
flavor: "azure",
1121+
isMonorepo: true,
1122+
hasLro: false,
1123+
generateReactNativeTarget: true
1124+
});
1125+
1126+
const initialPackageInfo = {
1127+
name: "@azure/test-package",
1128+
version: "1.0.0",
1129+
dependencies: {
1130+
"@azure-rest/core-client": "^2.3.1",
1131+
"@azure/core-rest-pipeline": "^1.20.0",
1132+
"tslib": "^2.8.1"
1133+
}
1134+
};
1135+
1136+
const packageFileContent = updatePackageFile(model, initialPackageInfo);
1137+
expect(packageFileContent).to.not.be.undefined;
1138+
const packageFile = JSON.parse(packageFileContent?.content ?? "{}");
1139+
1140+
// When opted-in, the `react-native` condition is added and must be
1141+
// positioned before `default` so Node's conditional resolution order
1142+
// matches the fresh-generation output in packageCommon.ts.
10911143
expect(packageFile).to.have.property("imports");
1092-
expect(packageFile.imports).to.have.property("#platform/*.js");
1144+
expect(packageFile.imports).to.deep.equal({
1145+
"#platform/*": {
1146+
browser: "./src/*-browser.mts",
1147+
"react-native": "./src/*-react-native.mts",
1148+
default: "./src/*.ts"
1149+
}
1150+
});
1151+
expect(
1152+
Object.keys(packageFile.imports["#platform/*"])
1153+
).to.deep.equal(["browser", "react-native", "default"]);
1154+
});
1155+
1156+
it("should NOT add react-native to platform imports for non-monorepo packages even if generateReactNativeTarget is true", () => {
1157+
// needsPlatformImportsUpdate requires azureSdkForJs (isMonorepo) to be true.
1158+
// For non-monorepo packages, the platform imports block must be skipped
1159+
// entirely regardless of generateReactNativeTarget.
1160+
const model = createMockModel({
1161+
moduleKind: "esm",
1162+
flavor: "azure",
1163+
isMonorepo: false,
1164+
hasLro: false,
1165+
generateReactNativeTarget: true
1166+
});
1167+
1168+
const initialPackageInfo = {
1169+
name: "@azure/test-package",
1170+
version: "1.0.0",
1171+
dependencies: {
1172+
"@azure/core-client": "^1.9.3",
1173+
"tslib": "^2.6.2"
1174+
}
1175+
};
1176+
1177+
const packageFileContent = updatePackageFile(model, initialPackageInfo);
1178+
expect(packageFileContent).to.not.be.undefined;
1179+
const packageFile = JSON.parse(packageFileContent?.content ?? "{}");
1180+
1181+
expect(packageFile).to.not.have.property("imports");
10931182
});
10941183

10951184
it("should migrate @azure/core-client for non-monorepo Azure packages", () => {

packages/typespec-ts/src/framework/hooks/binder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ class BinderImp implements Binder {
223223
* that has a polyfill variant (-browser.mts or -react-native.mts sibling),
224224
* or undefined if subpath imports are disabled or no variant exists.
225225
* e.g. "src/static-helpers/serialization/get-binary-response.ts"
226-
* -> "#platform/static-helpers/serialization/get-binary-response.js"
226+
* -> "#platform/static-helpers/serialization/get-binary-response"
227227
*/
228228
private getPlatformImportSpecifier(
229229
declarationSourceFile: SourceFile
@@ -242,7 +242,7 @@ class BinderImp implements Binder {
242242
);
243243
if (!hasBrowserVariant && !hasReactNativeVariant) return undefined;
244244
const relativePath = filePath.substring(srcIndex + "/src/".length);
245-
return "#platform/" + relativePath.replace(/\.ts$/, ".js");
245+
return "#platform/" + relativePath.replace(/\.ts$/, "");
246246
}
247247

248248
/**

packages/typespec-ts/src/framework/load-static-helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export async function loadStaticHelpers(
129129
!specifier.includes("-browser") &&
130130
!specifier.includes("-react-native")
131131
) {
132-
i.setModuleSpecifier("#platform/static-helpers/platform-types.js");
132+
i.setModuleSpecifier("#platform/static-helpers/platform-types");
133133
}
134134
}
135135
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { platformTag } from "./platform-types";
2+
3+
export function usesPlatformImport() {
4+
return platformTag;
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const platformTag = "platform";

packages/typespec-ts/test-next/integration/hooks/binder.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,45 @@ describe("Binder", () => {
393393
//
394394
// buildCsvCollection();
395395
});
396+
397+
it("should use #platform subpath imports without extension for helpers with platform variants", () => {
398+
binder = provideBinder(project, { useSubpathImports: true });
399+
const helperFile = project.createSourceFile(
400+
"src/static-helpers/serialization/get-binary-response.ts",
401+
"",
402+
{
403+
overwrite: true
404+
}
405+
);
406+
project.createSourceFile(
407+
"src/static-helpers/serialization/get-binary-response-browser.mts",
408+
"export {};",
409+
{
410+
overwrite: true
411+
}
412+
);
413+
addDeclaration(
414+
helperFile,
415+
{
416+
kind: StructureKind.Function,
417+
name: "getBinaryResponse"
418+
},
419+
"getBinaryResponse"
420+
);
421+
422+
const sourceFile = project.createSourceFile("src/test-platform.ts", "", {
423+
overwrite: true
424+
});
425+
sourceFile.addStatements(`${resolveReference("getBinaryResponse")}();`);
426+
427+
binder.resolveAllReferences("/modularPackageFolder/src");
428+
429+
assertGetImportStatements(
430+
sourceFile,
431+
"#platform/static-helpers/serialization/get-binary-response"
432+
);
433+
assertGetStatement(sourceFile, "getBinaryResponse();");
434+
});
396435
});
397436

398437
describe("External Dependencies Override", () => {

packages/typespec-ts/test-next/integration/load-static-files.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ describe("loadStaticHelpers", () => {
2626
const helperDeclarations = await loadStaticHelpers(project, helpers, {
2727
helpersAssetDirectory
2828
});
29-
expect(project.getSourceFiles()).to.toHaveLength(1);
29+
expect(
30+
project
31+
.getSourceFiles()
32+
.some((file) => file.getFilePath().endsWith("/static-helpers/utils.ts"))
33+
).toBe(true);
3034
const buildCsvCollectionDeclaration = helperDeclarations.get(
3135
refkey(helpers.buildCsvCollection)
3236
);
@@ -64,4 +68,33 @@ describe("loadStaticHelpers", () => {
6468
})
6569
).rejects.toThrowError(/invalid helper kind/);
6670
});
71+
72+
it("should rewrite platform-types imports to #platform subpath without extension for azure monorepo", async () => {
73+
const helpers = {
74+
usesPlatformImport: {
75+
kind: "function",
76+
name: "usesPlatformImport",
77+
location: "platform-import.ts"
78+
}
79+
} as const;
80+
81+
await loadStaticHelpers(project, helpers, {
82+
helpersAssetDirectory,
83+
options: {
84+
flavor: "azure",
85+
azureSdkForJs: true
86+
} as any
87+
});
88+
89+
const sourceFile = project
90+
.getSourceFiles()
91+
.find((file) =>
92+
file.getFilePath().endsWith("/static-helpers/platform-import.ts")
93+
);
94+
assert(sourceFile);
95+
const importDecl = sourceFile.getImportDeclaration(
96+
"#platform/static-helpers/platform-types"
97+
);
98+
expect(importDecl).toBeDefined();
99+
});
67100
});

0 commit comments

Comments
 (0)