From 7db109d3fa0a720232e3d8f4af40fcf4a41aa9ab Mon Sep 17 00:00:00 2001 From: Juliana Oliveira Date: Fri, 5 Aug 2022 15:00:04 -0300 Subject: [PATCH 1/2] fix(remix-dev): don't look for a tsconfig outside rootDir This patch addresses an issue[1] in which Remix looks for a tsconfig outside the root directory. We fix this behavior by looking for a tsconfig where it should be (in the rootDirectory itself) and passing its absolute path to `tsconfig-paths`. `tsconfig-paths` will then treat the given path as a tsconfig file path and won't crawl further. If no tsconfig file is found by us in our root directory, we simply won't call `tsconfig-paths` at all. Note that crawling up is `tsconfig-path`s intended behavior, but not Remix's. A secondary bug that causes this same issue is due to esbuild[2]. When no tsconfig is passed to esbuild, it will crawl the filesystem up looking for any tsconfig available and will load it. When tsconfig is explicitly set to `undefined`, it does the crawling anyways. Unfortunately, this is a fix that has to be done in `esbuild`, but a few alternatives to manage this unwanted behavior for now can be: 1. Create an empty tsconfig in the build directory and pass it to esbuild so it doesn't crawl outside rootDir; 2. Show a warning to let the user know why the crawling is happening. Fixes #3210 [1] https://github.com/remix-run/remix/issues/3210 [2] https://github.com/evanw/esbuild/issues/2440 Signed-off-by: Juliana Oliveira --- contributors.yml | 1 + packages/remix-dev/__tests__/readConfig-test.ts | 2 ++ packages/remix-dev/compiler.ts | 8 ++++++++ packages/remix-dev/compiler/plugins/mdx.ts | 2 +- .../compiler/plugins/serverBareModulesPlugin.ts | 2 +- .../remix-dev/compiler/utils/tsconfig/index.ts | 13 +++++++++++-- packages/remix-dev/config.ts | 15 +++++++++++++++ 7 files changed, 39 insertions(+), 4 deletions(-) diff --git a/contributors.yml b/contributors.yml index dbafe37c1fd..0f5d456367c 100644 --- a/contributors.yml +++ b/contributors.yml @@ -203,6 +203,7 @@ - juwiragiye - jveldridge - jvnm-dev +- jwnx - kalch - kanermichael - karimsan diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts index ddfed1ba54c..1b101f88624 100644 --- a/packages/remix-dev/__tests__/readConfig-test.ts +++ b/packages/remix-dev/__tests__/readConfig-test.ts @@ -21,6 +21,7 @@ describe("readConfig", () => { serverBuildPath: expect.any(String), assetsBuildDirectory: expect.any(String), relativeAssetsBuildDirectory: expect.any(String), + tsconfigPath: expect.any(String) }, ` Object { @@ -50,6 +51,7 @@ describe("readConfig", () => { "serverMode": "production", "serverModuleFormat": "cjs", "serverPlatform": "node", + "tsconfigPath": Any, "watchPaths": Array [], } ` diff --git a/packages/remix-dev/compiler.ts b/packages/remix-dev/compiler.ts index caf56acf16b..edd0abad67b 100644 --- a/packages/remix-dev/compiler.ts +++ b/packages/remix-dev/compiler.ts @@ -374,6 +374,10 @@ async function createBrowserBuild( sourcemap: options.sourcemap, metafile: true, incremental: options.incremental, + // As pointed out by https://github.com/evanw/esbuild/issues/2440, when tsconfig is set to + // `undefined`, esbuild will keep looking for a tsconfig.json recursively up. This unwanted + // behavior can only be avoided by creating an empty tsconfig file in the root directory. + tsconfig: config.tsconfigPath, mainFields: ["browser", "module", "main"], treeShaking: true, minify: options.mode === BuildMode.Production, @@ -463,6 +467,10 @@ function createServerBuild( loader: loaders, bundle: true, logLevel: "silent", + // As pointed out by https://github.com/evanw/esbuild/issues/2440, when tsconfig is set to + // `undefined`, esbuild will keep looking for a tsconfig.json recursively up. This unwanted + // behavior can only be avoided by creating an empty tsconfig file in the root directory. + tsconfig: config.tsconfigPath, incremental: options.incremental, sourcemap: options.sourcemap, // use linked (true) to fix up .map file // The server build needs to know how to generate asset URLs for imports diff --git a/packages/remix-dev/compiler/plugins/mdx.ts b/packages/remix-dev/compiler/plugins/mdx.ts index 2b0ec22e86a..91aeecda3c9 100644 --- a/packages/remix-dev/compiler/plugins/mdx.ts +++ b/packages/remix-dev/compiler/plugins/mdx.ts @@ -17,7 +17,7 @@ export function mdxPlugin(config: RemixConfig): esbuild.Plugin { ]); build.onResolve({ filter: /\.mdx?$/ }, (args) => { - let matchPath = createMatchPath(); + let matchPath = createMatchPath(config.tsconfigPath); // Resolve paths according to tsconfig paths property function resolvePath(id: string) { if (!matchPath) { diff --git a/packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts b/packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts index 4e9fcffe17f..d5124b0a7df 100644 --- a/packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts +++ b/packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts @@ -23,7 +23,7 @@ export function serverBareModulesPlugin( let isDenoRuntime = remixConfig.serverBuildTarget === "deno"; // Resolve paths according to tsconfig paths property - let matchPath = isDenoRuntime ? undefined : createMatchPath(); + let matchPath = isDenoRuntime ? undefined : createMatchPath(remixConfig.tsconfigPath); function resolvePath(id: string) { if (!matchPath) { return id; diff --git a/packages/remix-dev/compiler/utils/tsconfig/index.ts b/packages/remix-dev/compiler/utils/tsconfig/index.ts index 8836e7a472f..559a34e8800 100644 --- a/packages/remix-dev/compiler/utils/tsconfig/index.ts +++ b/packages/remix-dev/compiler/utils/tsconfig/index.ts @@ -2,8 +2,17 @@ import tsConfigPaths from "tsconfig-paths"; import { writeConfigDefaults } from "./write-config-defaults"; -export function createMatchPath() { - let configLoaderResult = tsConfigPaths.loadConfig(); +export function createMatchPath(tsconfigPath: string | undefined) { + // There is no tsconfig to match paths against. + if (!tsconfigPath) { + return undefined; + } + + // When passing a absolute path, loadConfig assumes that the path contains + // a tsconfig file. + // Ref.: https://github.com/dividab/tsconfig-paths/blob/v4.0.0/src/__tests__/config-loader.test.ts#L74 + let configLoaderResult = tsConfigPaths.loadConfig(tsconfigPath); + if (configLoaderResult.resultType === "failed") { if (configLoaderResult.message === "Missing baseUrl in compilerOptions") { throw new Error( diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index 72927be1a5f..9487d511576 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -268,6 +268,11 @@ export interface RemixConfig { * A list of directories to watch. */ watchPaths: string[]; + + /** + * The path for the tsconfig file, if present on the root directory. + */ + tsconfigPath: string | undefined; } /** @@ -436,6 +441,15 @@ export async function readConfig( let serverDependenciesToBundle = appConfig.serverDependenciesToBundle || []; + // When tsconfigPath is undefined, the default "tsconfig.json" is not + // found in the root directory. + let tsconfigDefaultFilename = "tsconfig.json" + let tsconfigPath: string | undefined = undefined + + if (fse.existsSync(path.resolve(rootDirectory, tsconfigDefaultFilename))) { + tsconfigPath = path.resolve(rootDirectory, tsconfigDefaultFilename) + } + return { appDirectory, cacheDirectory, @@ -458,6 +472,7 @@ export async function readConfig( serverDependenciesToBundle, mdx, watchPaths, + tsconfigPath, }; } From 3eeddf4e6608183fe563de9fd20cd0d5ae05fd10 Mon Sep 17 00:00:00 2001 From: Juliana Oliveira Date: Mon, 22 Aug 2022 12:26:12 -0300 Subject: [PATCH 2/2] fix(remix): consider jsconfig.json for `tsConfigPath` resolution Now we not only consider tsconfig.json, but also `jsconfig.json`. Signed-off-by: Juliana Oliveira --- packages/remix-dev/config.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index 6f7e7d5309c..39d87a89f29 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -457,11 +457,14 @@ export async function readConfig( // When tsconfigPath is undefined, the default "tsconfig.json" is not // found in the root directory. - let tsconfigDefaultFilename = "tsconfig.json" - let tsconfigPath: string | undefined = undefined - - if (fse.existsSync(path.resolve(rootDirectory, tsconfigDefaultFilename))) { - tsconfigPath = path.resolve(rootDirectory, tsconfigDefaultFilename) + let tsconfigPath: string | undefined; + let rootTsconfig = path.resolve(rootDirectory, "tsconfig.json"); + let rootJsConfig = path.resolve(rootDirectory, "jsconfig.json"); + + if (fse.existsSync(rootTsconfig)) { + tsconfigPath = rootTsconfig; + } else if (fse.existsSync(rootJsConfig)) { + tsconfigPath = rootJsConfig; } return {