Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(metro-plugin-typescript): respect root moduleSuffixes #2544

Merged
merged 1 commit into from
Jul 20, 2023
Merged

fix(metro-plugin-typescript): respect root moduleSuffixes #2544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-mirrors-provide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rnx-kit/metro-plugin-typescript": patch
---

Respect `moduleSuffixes` set at the root level
112 changes: 5 additions & 107 deletions packages/metro-plugin-typescript/src/resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,105 +2,10 @@ import { parseModuleRef } from "@rnx-kit/tools-node";
import ts from "typescript";
import type { ResolverContext } from "./types";

function intersection<T>(lhs: T[], rhs: T[]): T[] {
return lhs.filter((v) => rhs.includes(v));
}

function isEqual<T>(lhs: T[], rhs: T[]): boolean {
const length = lhs.length;
if (length !== rhs.length) {
return false;
}

for (let i = 0; i < length; ++i) {
if (lhs[i] !== rhs[i]) {
return false;
}
}

return true;
}

function isPackageRef(name: string): boolean {
return !parseModuleRef(name).path;
}

/**
* Get TypeScript compiler options with the `moduleSuffixes` property configured
* for the current React Native project. `moduleSuffixes` must contain all of the
* React Native platform file extensions -- in precedence order.
*
* Examine the given set of compiler options for the project. If `moduleSuffixes`
* is already set, and it does not contain the right React Native entries, we will
* throw an error. We cannot proceed because there's no way to **safely** add to
* `moduleSuffixes`. Additions change the way modules are resolved to files, and
* can only be done reliably by the package owner.
*
* When an error is thrown, it explains this, and offers some options to the developer.
*
* @param context Resolver context. Describes the current React Native project.
* @param moduleName Name of the module being resolved (used for error reporting).
* @param containingFile File containing the module reference (used for error reporting).
* @param options Compiler options for the module
* @returns Compiler options with a `moduleSuffixes` property containing the list of React Native platform file extensions. This may be input options object, or it may be a copy (if changes were made).
*/
export function getCompilerOptionsWithReactNativeModuleSuffixes(
context: ResolverContext,
moduleName: string,
containingFile: string,
options: ts.CompilerOptions
): ts.CompilerOptions {
if (!options.moduleSuffixes) {
// `moduleSuffixes` is not defined. Return a copy of the input object with
// `moduleSuffixes` set to the list of React Native platform extensions.
return {
...options,
moduleSuffixes: context.platformFileExtensions,
};
}

// `moduleSuffixes` is already defined in the module's compiler options.
// We cannot safely modify it because we don't know how to order the entries
// when adding those needed for React Native. Ordering matters because it controls
// file selection. e.g. If "ios" comes before "native", then "foo.ios.ts" will be
// resolved ahead of "foo.native.ts".
//
// Package authors may use a convention where they set 'moduleSuffixes' to [".native", ""].
// They do this when they don't have platform-specific code (e.g. iOS only). Instead,
// they use suffixes to separate React Native code (.native.ts) from Web code (.ts). We
// allow that convention as well.
//
// The best we can do is check `moduleSuffixes` for either of these conventions, and fail
// if they aren't met.

const hasAllPlatformFileExtensions = isEqual(
intersection(options.moduleSuffixes, context.platformFileExtensions),
context.platformFileExtensions
);

const nativePlatformFileExtension = [".native", ""];
const hasNativePlatformFileExtension = isEqual(
intersection(options.moduleSuffixes, nativePlatformFileExtension),
nativePlatformFileExtension
);
if (!hasAllPlatformFileExtensions && !hasNativePlatformFileExtension) {
const currentSuffixes = options.moduleSuffixes.join(",");
const neededAllSuffixes = context.platformFileExtensions.join(",");
const neededNativeSuffixes = nativePlatformFileExtension.join(",");
throw new Error(
`Failed to resolve module reference '${moduleName}' in source file '${containingFile}.\n` +
`The parent package has a TypeScript configuration which sets 'moduleSuffixes' to '${currentSuffixes}'.\n` +
`This is incompatible with the target platform '${context.platform}', which requires 'moduleSuffixes' to contain either '${neededAllSuffixes}' or just '${neededNativeSuffixes}', in order.\n` +
`We would like to understand any use cases where this error occurs, as there may be room to make improvements.\n` +
`Please add a comment about your scenario, and include this error message: https://github.com/microsoft/rnx-kit/discussions/1971.`
);
}

// Return the original compiler options, since we know they have the right
// `moduleSuffixes` entries for React Native resolution.
return options;
}

/**
* Resolves a module to a file using TypeScript's built-in module resolver and
* the `moduleSuffixes` compiler option.
Expand All @@ -126,12 +31,7 @@ export function resolveModuleName(
// suffixes as they are not used when looking at main fields.
const optionsWithSuffixes = isPackageRef(moduleName)
? options
: getCompilerOptionsWithReactNativeModuleSuffixes(
context,
moduleName,
containingFile,
options
);
: { ...options, moduleSuffixes: context.platformFileExtensions };

//
// Invoke the built-in TypeScript module resolver.
Expand Down Expand Up @@ -271,12 +171,10 @@ export function resolveTypeReferenceDirectives(
: typeDirectiveName.fileName.toLowerCase();

// Ensure the compiler options has `moduleSuffixes` set correctly for this RN project.
const optionsWithSuffixes = getCompilerOptionsWithReactNativeModuleSuffixes(
context,
name,
containingFile,
options
);
const optionsWithSuffixes = {
...options,
moduleSuffixes: context.platformFileExtensions,
};

//
// Invoke the built-in TypeScript type-reference resolver.
Expand Down
72 changes: 0 additions & 72 deletions packages/metro-plugin-typescript/test/resolver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type ts from "typescript";
import {
getCompilerOptionsWithReactNativeModuleSuffixes,
resolveModuleNames,
resolveTypeReferenceDirectives,
} from "../src/resolver";
Expand Down Expand Up @@ -67,77 +66,6 @@ describe("Resolver", () => {
expect(modules).toEqual(resolvedModules);
});

test("adds moduleSuffixes when it is not present", () => {
// platform is ios, and is set by context.platform. moduleSuffixes must be set
// to the list of ios-related extensions.
const inputOptions = {};
const outputOptions = getCompilerOptionsWithReactNativeModuleSuffixes(
context,
"module",
"containing-file",
inputOptions
);
expect(outputOptions).toEqual({
moduleSuffixes: [".ios", ".native", ""],
});
});

test("verifies that moduleSuffixes is valid when it is already present", () => {
// platform is ios, and is set by context.platform. moduleSuffixes must have
// all of the ios-related extensions, in order, but can have others mixed in.
const inputOptions = {
moduleSuffixes: [
".goobers",
".ios",
".milk-duds",
".lemonheads",
".native",
"",
".snickers",
],
};
const outputOptions = getCompilerOptionsWithReactNativeModuleSuffixes(
context,
"module",
"containing-file",
inputOptions
);
expect(outputOptions).toEqual({
moduleSuffixes: inputOptions.moduleSuffixes, // output must match input
});
});

test("verifies that moduleSuffixes is valid when it is already present without a platform name", () => {
// platform is ios, and is set by context.platform. moduleSuffixes must have
// all of the ios-related extensions, in order, but can have others mixed in.
const inputOptions = {
moduleSuffixes: [".lemonheads", ".native", "", ".snickers"],
};
const outputOptions = getCompilerOptionsWithReactNativeModuleSuffixes(
context,
"module",
"containing-file",
inputOptions
);
expect(outputOptions).toEqual({
moduleSuffixes: inputOptions.moduleSuffixes, // output must match input
});
});

test("fails when moduleSuffixes is set, but is invalid for the target platform", () => {
const inputOptions = {
moduleSuffixes: ["this isn't going to work"],
};
expect(() =>
getCompilerOptionsWithReactNativeModuleSuffixes(
context,
"module",
"containing-file",
inputOptions
)
).toThrowError();
});

test("resolves type reference directives", () => {
const mockResolveTypeReferenceDirective = jest.fn();
mockResolveTypeReferenceDirective.mockReturnValue({
Expand Down