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.

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

Build fails if there's a broken tsconfig.json file outside the project #3210

Closed
machour opened this issue May 17, 2022 · 7 comments · Fixed by #3936
Closed

Build fails if there's a broken tsconfig.json file outside the project #3210

machour opened this issue May 17, 2022 · 7 comments · Fixed by #3936
Labels
bug Something isn't working

Comments

@machour
Copy link
Collaborator

machour commented May 17, 2022

What version of Remix are you using?

latest

Steps to Reproduce

  • npx create-remix@latest (just the basics, typescript)
  • cd my-remix-app
  • touch ../tsconfig.json
  • npm run dev

Expected Behavior

The build should work fine, and the tsconfig.json file outside my project should be ignored

Actual Behavior

🍺 ~/my-remix-app npm run dev

dev
remix dev

Watching Remix app in development mode...

✘ [ERROR] Unexpected end of file

../tsconfig.json:1:0:
  1 │
    ╵ ^

Build failed with 1 error:
../tsconfig.json:1:0: ERROR: Unexpected end of file

@machour
Copy link
Collaborator Author

machour commented May 17, 2022

@F3n67u 👋🏼
Could this be caused by tsconfig-paths ?

@F3n67u
Copy link
Contributor

F3n67u commented May 17, 2022

@F3n67u 👋🏼
Could this be caused by tsconfig-paths ?

I think yes. tsconfig-paths will walk to parent directory until to root to find a tsconfig.json. if the found tsconfig.json is not valid, then error will throw when parse the invalid json config. I think error message could be improved.

@machour
Copy link
Collaborator Author

machour commented May 17, 2022

Can we tell it to stop searching when reaching the remix app root?
I do have a valid tsconfig.json file that was generated by create-remix, so I don't see why it continues looking up the path

@nivekithan
Copy link

nivekithan commented May 17, 2022

I don't think there is any problem with tsconfig-paths, rather I think problem is with esbuild. Setting tsconfig key in esbuild.build function on both createServerBuild and createBrowserBuild have solved error for me.

EDIT: This is causing lot of test to fail on running yarn test

async function createBrowserBuild(
  config: RemixConfig,
  options: BuildOptions & { incremental?: boolean }
): Promise<esbuild.BuildResult> {
  ...
  ...
  ...
  return esbuild.build({
    ...
    tsconfig: `${config.rootDirectory}/tsconfig.json`, // Setting tsconfig explicitly
  });
}
function createServerBuild(
  config: RemixConfig,
  options: Required<BuildOptions> & { incremental?: boolean },
  assetsManifestPromiseRef: AssetsManifestPromiseRef
): Promise<esbuild.BuildResult> {
  ...
  ...
  ...
  
  return esbuild
    .build({
      ...
      tsconfig: `${config.rootDirectory}/tsconfig.json`,
    })
    .then(...);
}

@F3n67u
Copy link
Contributor

F3n67u commented May 17, 2022

I think yes. tsconfig-paths will walk to parent directory until to root to find a tsconfig.json. if the found tsconfig.json is not valid, then error will throw when parse the invalid json config. I think error message could be improved.

@machour I am wrong a lot. after some debugging, I found it's not a problem with tsconfig-paths;

$ npx create-remix@latest (just the basics, typescript)
$ cd my-remix-app
$ touch ../tsconfig.json
$ npm run dev

> dev
> remix dev

Watching Remix app in development mode...
config loader result {
  resultType: 'success',
  configFileAbsolutePath: '/Users/feng/test/my-remix-app/tsconfig.json',
  baseUrl: '.',
  absoluteBaseUrl: '/Users/feng/test/my-remix-app',
  paths: { '~/*': [ './app/*' ] }
}
✘ [ERROR] [plugin browser-route-module] Build failed with 1 error:
../tsconfig.json:1:0: ERROR: Unexpected end of file

 ✘ [ERROR] [plugin browser-route-module] Build failed with 1 error:
../tsconfig.json:1:0: ERROR: Unexpected end of file


Build failed with 2 errors:
error: Build failed with 1 error:
../tsconfig.json:1:0: ERROR: Unexpected end of file
error: Build failed with 1 error:
../tsconfig.json:1:0: ERROR: Unexpected end of file
💿 Built in 486ms

config loader result log is added by me after let configLoaderResult = loadTsConfig();:

https://github.com/remix-run/remix/blob/main/packages/remix-dev/compiler/utils/tsconfig/index.ts#L8

the log result shows tsconfig-paths does find the correct tsconfig.json: /Users/feng/test/my-remix-app/tsconfig.json, but not the empty(invalid) tsconfig.json which is at /Users/feng/test/tsconfig.json

@MichaelDeBoey
Copy link
Member

MichaelDeBoey commented May 17, 2022

Looking at the error message, it seems to be an error in our own browserRouteModulesPlugin (haven't really checked it though)

https://github.com/remix-run/remix/blob/15d18e624e9de6fe22cc44db8db7e509d218fc4e/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts

jwnx added a commit to jwnx/remix that referenced this issue Aug 5, 2022
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>
@MichaelDeBoey
Copy link
Member

Closed by #3936

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants