Skip to content

Commit

Permalink
fix(remix-dev): don't look for a tsconfig outside rootDir
Browse files Browse the repository at this point in the history
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 remix-run#3210

[1] remix-run#3210
[2] evanw/esbuild#2440

Signed-off-by: Juliana Oliveira <juliana@uma.ni>
  • Loading branch information
jwnx committed Aug 5, 2022
1 parent c0cf73a commit 7db109d
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 4 deletions.
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@
- juwiragiye
- jveldridge
- jvnm-dev
- jwnx
- kalch
- kanermichael
- karimsan
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe("readConfig", () => {
serverBuildPath: expect.any(String),
assetsBuildDirectory: expect.any(String),
relativeAssetsBuildDirectory: expect.any(String),
tsconfigPath: expect.any(String)
},
`
Object {
Expand Down Expand Up @@ -50,6 +51,7 @@ describe("readConfig", () => {
"serverMode": "production",
"serverModuleFormat": "cjs",
"serverPlatform": "node",
"tsconfigPath": Any<String>,
"watchPaths": Array [],
}
`
Expand Down
8 changes: 8 additions & 0 deletions packages/remix-dev/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-dev/compiler/plugins/mdx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions packages/remix-dev/compiler/utils/tsconfig/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 15 additions & 0 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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,
Expand All @@ -458,6 +472,7 @@ export async function readConfig(
serverDependenciesToBundle,
mdx,
watchPaths,
tsconfigPath,
};
}

Expand Down

0 comments on commit 7db109d

Please sign in to comment.