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.

Sign up for GitHub

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

Jump to bottom

"Module not found" run-time error, if the project directory name contains non-ASCII characters #1868

Closed
3 of 4 tasks
retnag opened this issue Mar 26, 2022 · 4 comments · May be fixed by #2745
Closed
3 of 4 tasks

"Module not found" run-time error, if the project directory name contains non-ASCII characters #1868

retnag opened this issue Mar 26, 2022 · 4 comments · May be fixed by #2745
Labels
bug needs triage

Comments

@retnag
Copy link

retnag commented Mar 26, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

I get following error when I start the server:

$ npm start                 
> server@0.0.1 start
> nest start

Error: Cannot find module '/home/xyz/Documents/Programozás/2022/project/src/AType'
Require stack:
- /home/xyz/Documents/Programozás/2022/project/dist/app.controller.js
- /home/xyz/Documents/Programozás/2022/project/app.module.js
- /home/xyz/Documents/Programozás/2022/project/main.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
    at Function.Module._load (internal/modules/cjs/loader.js:746:27)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:101:18)
    at Object.<anonymous> (/home/xyz/Documents/Programozás/2022/project/dist/app.controller.js:31:46)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)

Minimum reproduction code

https://github.com/retnag/nestjs-swagger-apostrophe-in-path-error

Steps to reproduce

  1. Create a Directory, that contains a non-ASCII character in its name (I have tested it with "é" and "á". Example: "Programozás")
  2. cd into it, and clone the reproduction code into it. The goal here is, to have a non-ASCII character in the full path of the projects Filenames.
  3. run npm i as usual
  4. npm start

Key requirements provided by the linked repo:

  1. The full Path of where the project resides contains a non-ASCII character.
  2. The swagger cli-plugin is enabled
  3. The explicit or inferred return-type of a controller function, that handles an endpoint, is a class imported from another file in the project.

Expected behavior

I would expect the server to start without the error. I spent hours figuring out what's going on here, tests ran fine. Typescript compilation runs fine. I have tracked down the error to the following line in the emitted code by typescript (file: dist/app.controller.js:31):

openapi.ApiResponse({ status: 201, type: require("/home/xyz/Documents/Programoz\u00E1s/2022/project/src/AType").AType })

Notice the \u00E1s Unicode escape sequence in the file path string. Now the "Module not found" error makes sense, since this directory really doesn't exist....

Also, and this might be a separate Issue altogether, I would expect a relative path here, as this absolute path is only going to be valid on the developer machine, ergo the build will fail with this error on any other machine It's deployed to.

Package version

5.2.1

NestJS version

8.0.0

Node.js version

14.19.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

This was successfully reproduced on another machine on Windows.

I guess this is not a high-priority issue, as a simple workaround would be not to use Unicode characters in your directory structure, but guys, its 2022 :'), things like this really shouldn't be an issue anymore, I just lost hours debugging this, as I never had suspected a user error, or a circular dependency somwhere. Please fix this.

BUT: the fact that there is a full, dev-environment relevant absolute path emittet there, which will break the server in a production environment, kinda seems like a dealbreaker to me!
@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@psam44
Copy link

psam44 commented Apr 10, 2023

I experienced the same issue, with a 'é' character in the path leading to the project.
With a replacement by a simple 'e', the module name is back to an absolute form, as expected:
openapi.ApiResponse({ status: 200, type: require("./device.entity").Device }),

Windows, Nestjs 9.2.1, Node 18.13.0

@psam44
Copy link

psam44 commented Apr 11, 2023

The issue is located in lib/plugin/utils/plugin-utils.ts, function replaceImportPath(), called in lib/plugin/visitors/controller-class.visitor.ts.
Seen with a console.log(), parameter typeReference is given as import("/home/xyz/Documents/Programoz\u00E1s/2022/project/src/AType").AType.
For a string literal in a test script, escape it as 'import("/home/xyz/Documents/Programoz\\u00E1s/2022/project/src/AType").AType'.
Because of a call to convertPath(), the importPath variable becomes: ("/home/xyz/Documents/Programoz/u00E1s/2022/project/src/AType". Note the conversion of the backslash to a slash, introducing an invalid folder level, but mainly modifying the contents.
And after the slice(): /home/xyz/Documents/Programoz/u00E1s/2022/project/src/AType.
This doesn't prevent the relativePath variable to be built, even with a lot of ../ levels.
BUT the typeReference.replace() has no effect, because the pattern is no more in the original string.

The conversions in convertPath:

  • remove prefix //?/ ; where this pattern can come from? Should be at least documented with a comment.
  • \ -> / ; ok, usual on Windows platform, but the provided string has already normalized separator.
  • reduce 2 or more / by only one occurrence ; how can it happen? Should be at least documented with a comment.

Anyway, as is the current implementation, if there is any change in the importPath variable, necessary or not, the final replace() will be no-op.

Not tested, but if the fileName parameter is also given as /home/xyz/Documents/Programoz\u00E1s/2022/project/src/app.controller.js, then just removing the call to convertPath should allow to obtain the expected result require("./AType").AType.

(note: I won't provide a fix/PR)

Hannes-Kunnen added a commit to Hannes-Kunnen/nestjs-swagger that referenced this issue Dec 6, 2023
@Hannes-Kunnen
fix(core): replace unicode with UTF-8 character
487e3c0 
Replace escaped unicode occurrences in text with their respective UTF-8
character. This enables the use of non-ASCII characters in the project
path.

Closes nestjs#1868
Hannes-Kunnen added a commit to Hannes-Kunnen/nestjs-swagger that referenced this issue Dec 7, 2023
@Hannes-Kunnen
fix(core): unescape unicode in filenames
ceffc5e 
Unescape escaped unicode characters in file paths.
Filepaths returned from TypeScript have their non-ASCII characters
escaped.
See https://www.github.com/microsoft/TypeScript/issues/36174

This enables the use of non-ASCII characters in the project
path.

Closes nestjs#1868
@Hannes-Kunnen Hannes-Kunnen mentioned this issue Dec 7, 2023
Open
12 tasks
@kamilmysliwiec
Copy link
Member

Let's track this here #2745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug needs triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): support non-ASCII characters in filename paths Hannes-Kunnen/nestjs-swagger
3 participants
@psam44 @retnag @kamilmysliwiec