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

Remix updates tsconfig and doesn't care about extends key #4978

Closed
dormammun opened this issue Dec 30, 2022 · 18 comments
Closed

Remix updates tsconfig and doesn't care about extends key #4978

dormammun opened this issue Dec 30, 2022 · 18 comments

Comments

@dormammun
Copy link

dormammun commented Dec 30, 2022

What version of Remix are you using?

1.9.0

Steps to Reproduce

  1. Create tsconfig-base.json to extends
{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Default",
  "compilerOptions": {
    "target": "ES2019",
    "composite": false,
    "declaration": true,
    "declarationMap": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "inlineSources": false,
    "isolatedModules": true,
    "moduleResolution": "node",
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "noImplicitAny": false,
    "preserveWatchOutput": true,
    "skipLibCheck": true,
    "strict": true,
    "noEmit": true,
    "resolveJsonModule": true
  }
}
  1. Create tsconfig-react.json
{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "React Application",
  "extends": "./tsconfig-base.json",
  "compilerOptions": {
    "jsx": "react-jsx",
    "lib": ["DOM", "DOM.Iterable", "ES2019"],
    "module": "ESNext"
  },
  "include": ["src"]
}
  1. Create tsconfig-remix.json
{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Remix Application",
  "extends": "./tsconfig-react.json",
  "compilerOptions": {
    "allowJs": true,
    "resolveJsonModule": true
  }
}
  1. Update tsconfig.json
{
  "extends": "./tsconfig-remix.json",
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "@app/*": ["./app/*"]
    }
  }
}
  1. Run npm run dev
    And you'll see:
    The following suggested values were added to your "tsconfig.json". These values can be changed to fit your project's needs:

     - compilerOptions.forceConsistentCasingInFileNames was set to 'true'
     - compilerOptions.lib was set to 'DOM,DOM.Iterable,ES2019'
     - compilerOptions.strict was set to 'true'
     - compilerOptions.target was set to 'ES2019'
    

The following mandatory changes were made to your tsconfig.json:

    - compilerOptions.esModuleInterop was set to 'true'
    - compilerOptions.isolatedModules was set to 'true'
    - compilerOptions.jsx was set to 'react-jsx'
    - compilerOptions.noEmit was set to 'true'
    - compilerOptions.moduleResolution was set to 'node'

And tsconfig.json will be updated. So Remix doesn't care about extends and update config to

{
  "extends": "@packages/ts-configs/remix",
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "@app/*": ["./app/*"]
    },
    // everything that above added by remix after npm run dev
    "allowJs": true,
    "forceConsistentCasingInFileNames": true,
    "lib": ["DOM", "DOM.Iterable", "ES2019"],
    "strict": true,
    "target": "ES2019",
    "esModuleInterop": true,
    "isolatedModules": true,
    "jsx": "react-jsx",
    "noEmit": true,
    "resolveJsonModule": true,
    "moduleResolution": "node"
  }
}

As you see this props already exists in extended config, but why remix updates it?

If I run tsc --showConfig before npm run dev and config will be updated, I got this:

> tsc --showConfig

{
    "compilerOptions": {
        "target": "es2019",
        "composite": false,
        "declaration": true,
        "declarationMap": true,
        "esModuleInterop": true,
        "forceConsistentCasingInFileNames": true,
        "inlineSources": false,
        "isolatedModules": true,
        "moduleResolution": "node",
        "noUnusedLocals": false,
        "noUnusedParameters": false,
        "noImplicitAny": false,
        "preserveWatchOutput": true,
        "skipLibCheck": true,
        "strict": true,
        "noEmit": true,
        "resolveJsonModule": true,
        "jsx": "react-jsx",
        "lib": [
            "dom",
            "dom.iterable",
            "es2019"
        ],
        "module": "esnext",
        "allowJs": true,
        "baseUrl": "./",
        "paths": {
            "@app/*": [
                "./app/*"
            ]
        }
    },
    "files": [
        "./remix.env.d.ts",
        "./app/entry.client.tsx",
        "./app/entry.server.tsx",
        "./app/root.tsx",
        "./app/routes/index.tsx"
    ],
    "include": [
        "remix.env.d.ts",
        "**/*.ts",
        "**/*.tsx"
    ]
}

As you see, ts tracks extended configs and merged it.

Expected Behavior

Remix will track extends from tsconfig.json and will not update it

Actual Behavior

Remix doesn't care about tsconfig extends props.

If I log console.log(fullConfig); from
node_modules/@remix-run/dev/dist/compiler/utils/tsconfig/write-config-defaults.js
It returns

{
  extends: '@packages/ts-configs/remix',
  include: [ 'remix.env.d.ts', '**/*.ts', '**/*.tsx' ],
  compilerOptions: {
    baseUrl: '.',
    paths: { '@app/*': [Array] },
    allowJs: true,
    resolveJsonModule: true
  }
}
@mcansh
Copy link
Collaborator

mcansh commented Dec 30, 2022

interesting, we should be merging based on extends (https://github.com/remix-run/remix/blob/main/integration/tsconfig-test.ts#L94) - i'll take a look :)

@mcansh
Copy link
Collaborator

mcansh commented Dec 30, 2022

even weirder, just copy pasted the above tsconfigs and mine didn't update anything 😵‍💫

@dormammun
Copy link
Author

dormammun commented Dec 30, 2022

@mcansh Interesting: ) Added video

Screen-Recording-2022-12-31-at-0.mp4

@dormammun
Copy link
Author

dormammun commented Dec 30, 2022

@mcansh And added video with debugging where it's happens.

Screen-Recording-2022-12-31-at-0.mp4

Checking tsconfig with tsc --showConfig

Screen.Recording.2022-12-31.at.00.44.03.mov

@dormammun
Copy link
Author

@mcansh
Copy link
Collaborator

mcansh commented Dec 30, 2022

@mcansh

You can try it by https://github.com/md-gravity/Video-Chat-Application/tree/add-service-for-rooms-management

cool, i'll check it out asap

@dormammun
Copy link
Author

dormammun commented Dec 31, 2022

@mcansh
I checked it without monorepo, and it works as expected. But when I extends from node_modules aka monorepo, for some reason it doesn't resolve extends.

@dormammun
Copy link
Author

@mcansh I moved from npm workspaces to pnpm and the issue was resolved. Seems npm handles it different and remix in this way can't find packages from monorepo.

Thank you for your attitude.

@rhefner
Copy link

rhefner commented Jan 10, 2023

@dormammun @mcansh Isn't this a valid bug? It seems to me that if extends resolution in pnpm (assuming workspaces) works but the resolution in npm workspaces doesn't, then a fix would be warranted for npm workspaces. I ask as I've been running into the same thing in a yarn v3 workspaces repo when I use a package @myorg/typescript-config in "extends":

{
  "extends": "@myorg/typescript-config/tsconfig.remix.json"
}

If @myorg/typescript-config exists in the same monorepo and I instead use a relative path to the tsconfig, such as:

{
  "extends": "../../config/tsc/tsconfig.remix.json"
}

...Then it's all good. However, if/when we decide to rely on an external package instead, then I think we'd probably run into the same thing?

@mcansh
Copy link
Collaborator

mcansh commented Jan 10, 2023

definitely a valid bug, we use tsconfig-paths to handle tsconfig loading and it seems that it's returning the incorrect path when hitting extends in a npm workspace

{ 
  extendedConfig: '@packages/ts-configs/remix',
  extendedConfigPath: '/Users/logan/remix-npm-monorepo-tsconfig/my-remix-app/@packages/ts-configs/remix.json'
}

where @packages/ts-configs/remix.json is actually located at /Users/logan/remix-npm-monorepo-tsconfig/my-remix-app/packages/tsconfig/remix.json

...Then it's all good. However, if/when we decide to rely on an external package instead, then I think we'd probably run into the same thing?

{
  "extends": "../node_modules/@packages/ts-configs/remix.json"
}

@rhefner
Copy link

rhefner commented Jan 10, 2023

@mcansh Thank you for verifying! It's definitely a problem with yarn v2+ as well as I mentioned. 👍🏻

@rhefner
Copy link

rhefner commented Jan 13, 2023

@mcansh This needs to be reopened.

@mcansh mcansh reopened this Jan 13, 2023
@mcansh
Copy link
Collaborator

mcansh commented Jan 13, 2023

@mcansh This needs to be reopened.

agreed - gonna look into a barebones typescript project with yarn berry (and npm) and tsconfig-paths

@decanTyme
Copy link

I have no idea why but when I also extend from @tsconfig/remix it also still updates it. @tsconfig/remix should have the defaults from the remix dev logs set already:

// @tsconfig/remix
{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Remix",
  "compilerOptions": {
    "lib": ["DOM", "DOM.Iterable", "ES2019"],
    "isolatedModules": true,
    "esModuleInterop": true,
    "jsx": "react-jsx",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "target": "ES2019",
    "strict": true,
    "allowJs": true,
    "forceConsistentCasingInFileNames": true,
    "baseUrl": ".",
    "paths": {
      "~/*": ["./app/*"]
    },

    // Remix takes care of building everything in `remix build`.
    "noEmit": true
  }
}

@mcansh
Copy link
Collaborator

mcansh commented Mar 12, 2023

just tried a fresh app using @tsconfig/remix and it worked for yarn (v1), npm, and pnpm. for yarn and npm i had to re-add compilerOptions.paths in order for aliases to work, but no changes were made to my tsconfig from remix build or remix dev

{
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "extends": "@tsconfig/remix/tsconfig.json"
}

@decanTyme
Copy link

That's strange. Running tsc --showConfig with the tsconfig:

{
  "extends": "@tsconfig/remix",
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "compilerOptions": {
    "baseUrl": ".",
    "skipLibCheck": true
  }
}

I get:

{
    "compilerOptions": {
        "lib": [
            "dom",
            "dom.iterable",
            "es2019"
        ],
        "isolatedModules": true,
        "esModuleInterop": true,
        "jsx": "react-jsx",
        "moduleResolution": "node",
        "resolveJsonModule": true,
        "target": "es2019",
        "strict": true,
        "allowJs": true,
        "forceConsistentCasingInFileNames": true,
        "baseUrl": "./",
        "paths": {
            "~/*": [
                "./app/*"
            ]
        },
        "noEmit": true,
        "skipLibCheck": true
    },
    "files": [
        "./remix.env.d.ts",
        "./app/root.tsx",
        "./app/routes/_index.tsx"
    ],
    "include": [
        "remix.env.d.ts",
        "**/*.ts",
        "**/*.tsx"
    ]
}

... in also a fresh app, it is resolved exactly the same as when having "extends": "@tsconfig/remix/tsconfig.json". So I would assume having "extends": "@tsconfig/remix" would be fine and Remix shouldn't have any need to update it. But yes, having the actual file /tsconfig.json specified does work and there would be no change.

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-2e9c413-20230502 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.16.1-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants