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

Fix: Windows tsconfig.json resolution. aka: Typescript Project detection #14088

Closed
wants to merge 2 commits into from
Closed

Fix: Windows tsconfig.json resolution. aka: Typescript Project detection #14088

wants to merge 2 commits into from

Conversation

psyirius
Copy link

@psyirius psyirius commented Aug 12, 2022

Finding tsconfig.json is failed due to different path separators.

What does it do?

Normalizes the found tsconfig.json with the platform's own path separator which helps to succeed the later condition.

Why is it needed?

Throws Unknown dialect error when using TS on windows.
it's because due to the following bug it is not detected as a TS project and the ts config files are not loaded.
thus it recieves undefined as dialect.

On windows, the path separator is \ whereas on other platforms it is /.

In the below function:

module.exports = (dir, { filename = DEFAULT_TS_CONFIG_FILENAME, ancestorsLookup = false } = {}) => {
const dirAbsolutePath = path.resolve(dir);
const configFilePath = ts.findConfigFile(dirAbsolutePath, ts.sys.fileExists, filename);
if (!configFilePath || ancestorsLookup) {
return configFilePath;
}
return configFilePath.startsWith(dirAbsolutePath) ? configFilePath : undefined;
};

When getting dirAbsolutePath from dir, the path.resolve function normalizes the path with the platform path separator.
But typescript's function ts.findConfigFile normalizes the path with Posix-style path separator.
Unless we normalize both paths dirAbsolutePath and configFilePath to the same style it is never assured to match on all platforms even-though the tsconfig is found by typescript.

How to test it?

Init a project with typescript support and run strapi develop or strapi build && strapi start.
You'll be greeted with the below message...

$ strapi develop
Building your admin UI with development configuration...
Admin UI built successfully
[2022-08-12 10:58:14.772] debug: ⛔️ Server wasn't able to start properly.
[2022-08-12 10:58:14.773]error: Unknown dialect undefined
Error: Unknown dialect undefined
    at getDialectClass (D:\dev\strap\node_modules\@strapi\database\lib\dialects\index.js:12:13)
    at getDialect (D:\dev\strap\node_modules\@strapi\database\lib\dialects\index.js:19:23)
    at new Database (D:\dev\strap\node_modules\@strapi\database\lib\index.js:27:20)
    at Function.Database.init (D:\dev\strap\node_modules\@strapi\database\lib\index.js:77:33)
    at Strapi.bootstrap (D:\dev\strap\node_modules\@strapi\strapi\lib\Strapi.js:392:30)
    at Strapi.load (D:\dev\strap\node_modules\@strapi\strapi\lib\Strapi.js:457:16)
    at async Strapi.start (D:\dev\strap\node_modules\@strapi\strapi\lib\Strapi.js:198:9)

My package.json for reference:

{
  "name": "strap",
  "private": true,
  "version": "0.1.0",
  "description": "A Strapi application",
  "scripts": {
    "develop": "strapi develop",
    "start": "strapi start",
    "build": "strapi build",
    "strapi": "strapi"
  },
  "devDependencies": {},
  "dependencies": {
    "@strapi/strapi": "4.3.4",
    "@strapi/plugin-users-permissions": "4.3.4",
    "@strapi/plugin-i18n": "4.3.4",
    "better-sqlite3": "7.4.6"
  },
  "engines": {
    "node": ">=12.x.x <=16.x.x",
    "npm": ">=6.0.0"
  },
  "license": "MIT"
}

Related issue(s)/PR(s)

Possible Issue: #13237

finding tsconfig.json failed due to different path separators.
@strapi-cla
Copy link

strapi-cla commented Aug 12, 2022

CLA assistant check
All committers have signed the CLA.

@Refrezsh
Copy link

I encountered the same problem. In 4.3.2 everything worked in both Win and Linux environments, in 4.3.3 - 4.3.4 the same error. When creating db.connection it comes {}.

@psyirius
Copy link
Author

psyirius commented Aug 13, 2022

I encountered the same problem. In 4.3.2 everything worked in both Win and Linux environments, in 4.3.3 - 4.3.4 the same error. When creating db.connection it comes {}.

It's because your configs (which are in ts) aren't transpiled and loaded cuz the project isn't detected as a TS project.
Not sure what changed from previous versions. Haven't used any previous versions of strapi personally.
My suspect is the typescript lib which might have introduced a breaking change.

@antixrist
Copy link

It's also can be fixes by replace node's native path module:

const path = require('path');

to pathe package:

const path = require('pathe');

@florianmrz
Copy link
Contributor

Related issue #14079 and PR #14080

@psyirius psyirius changed the title Fix: Windows tsconfig.json find fix. Fix: Windows tsconfig.json resolution. aka: Typescript Project detection Aug 19, 2022
@psyirius psyirius changed the title Fix: Windows tsconfig.json resolution. aka: Typescript Project detection Fix: Windows tsconfig.json resolution. aka: Typescript Project detection Aug 19, 2022
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question

@moritzln
Copy link

moritzln commented Sep 5, 2022

Can somebody please review this? Currently new projects with typescript won't run. This fix worked for me.

@antokhio
Copy link

antokhio commented Sep 7, 2022

Fiiix pls

@innerdvations innerdvations added source: typescript Source is related to TypeScript (typings, tooling, ...) and removed source: core:strapi Source is core/strapi package labels Sep 8, 2022
@antokhio
Copy link

Hi can we finally have this merged? So we can update ts projects without patch-package

@humblearab
Copy link

Hello. Any merge updates?

Replacing const path = require('path'); with const path = require('pathe'); fixed the issue for me on Windows 11

@Convly
Copy link
Member

Convly commented Sep 20, 2022

Hey @psyirius, could you update your branch with main so that we can move forward with this, please?

@South-Paw
Copy link

South-Paw commented Oct 1, 2022

Wait ... I just looked at the date on this PR. You're telling me new Windows + TypeScript projects have been broken for over a month now with a fix proposed and no one on the Strapi team has moved to merge or fix this themselves yet?

@TheRealFlyingCoder
Copy link

Coming here to bump this as it should definitely be a priority, it is essentially blocking every new user on Windows who uses Typescript

Confirming the proposed fix works as well, with a quick little patch-package

@Bassel17
Copy link
Member

Bassel17 commented Oct 5, 2022

Thank you a lot for your contribution to this Pr,
we decide to move forward with the fix provided in this pr #14521

@Bassel17 Bassel17 closed this Oct 5, 2022
@psyirius
Copy link
Author

Hey @psyirius, could you update your branch with main so that we can move forward with this, please?

Oopsie, I stopped checking this pr after checking it for may days for update and no progress.
Glad to hear a fix is landed finally! Patching strapi on production was a nightmare on every maintenance.

@psyirius psyirius deleted the ts-config-path-norm-fix branch October 13, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: typescript Source is related to TypeScript (typings, tooling, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet