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

[BUG] tsed/di v7 does not work with new version of @tsed/logger (v7) #2917

Closed
SavvasVincentTamosaitis opened this issue Dec 4, 2024 · 10 comments
Assignees
Labels

Comments

@SavvasVincentTamosaitis
Copy link

SavvasVincentTamosaitis commented Dec 4, 2024

Describe the bug

Note: The title is my best guess about what is going on based on the errors below and observing the diffs between package-lock files resulting in working and broken builds.

After deleting old package-lock or similar, using @tsed/di as import, I get an error when testing with jest (on the files that include tsed/di import) and when starting with node. Note: I do not import logger for my project.

Testing the files with jest gives me this error

/root/node_modules/@tsed/logger/lib/esm/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export * from "./common/index.js";

    SyntaxError: Unexpected token 'export'

    > 1 | import { Service } from '@tsed/di';

Starting with node ("start": "node dist/index.js",) gives me:

/root/node_modules/@tsed/di/lib/cjs/node/domain/ContextLogger.js:85

Error [ERR_REQUIRE_ESM]: require() of ES Module /root/node_modules/@tsed/logger/lib/esm/index.js from /root/node_modules/@tsed/di/lib/cjs/node/domain/ContextLogger.js not supported.
Instead change the require of index.js in /root/node_modules/@tsed/di/lib/cjs/node/domain/ContextLogger.js to a dynamic import() which is available in all CommonJS modules.
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/di/lib/cjs/node/domain/ContextLogger.js:5:18)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/di/lib/cjs/node/index.js:8:22)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/di/lib/cjs/index.js:5:22)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/common/lib/cjs/builder/PlatformBuilder.js:28:14)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/common/lib/cjs/index.js:7:22)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/platform-express/lib/cjs/components/PlatformExpress.js:6:18)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at Object.<anonymous> (/root/node_modules/@tsed/platform-express/lib/cjs/index.js:7:22)
    at Hook.Module.require (/root/node_modules/dd-trace/packages/dd-trace/src/ritm.js:85:33)
    at @tsed/platform-express (/root/dist/index.js:2:8815)
    at __webpack_require__ (/root/dist/index.js:2:9432)
    at /root/dist/index.js:2:9672
    at /root/dist/index.js:2:10112
    at Object.<anonymous> (/root/dist/index.js:2:10116) {
  code: 'ERR_REQUIRE_ESM'
}

To Reproduce

Attempt to start an app that uses tsed/di (^7.0.0).

I suspect this may also be an issue for other tsed package in v7 which are dependent on "@tsed/logger": ">=6.7.5" as well.

Expected behavior

Errors do not occur when starting an app with v7 tsed dependencies that are dependent on @tsed/logger

Code snippets

No response

Repository URL example

No response

OS

macOS

Node version

v18.20.4 and v20.17.0

Library version

v7.84.1

Additional context

No response

@SavvasVincentTamosaitis
Copy link
Author

I've tested directly editing package-lock to use older version for tsed logger and reinstalled dependencies after only these changes. With this change the errors are gone, so it does seem to be an issue with tsed logger.

I changed >=6.7.5 for all tsed related dependencies to ^6.7.5:

        "@tsed/logger": "^6.7.5",
        "@tsed/logger-file": "^6.7.5",

@SavvasVincentTamosaitis
Copy link
Author

This also affects version 6 (6.133.1).

@Romakita
Copy link
Collaborator

Romakita commented Dec 5, 2024

Tsed logger v7 only support esm eco system. Please read the release not: https://github.com/tsedio/logger/releases/tag/v7.0.0

For tsed v7, use v6 logger version. I close this issue!

@Romakita Romakita closed this as completed Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

🎉 Are you happy?

If you appreciated the support, know that it is free and is carried out on personal time ;)

A support, even a little bit makes a difference for me and continues to bring you answers!

github opencollective

@Romakita
Copy link
Collaborator

Romakita commented Dec 5, 2024

I can fix the di package.json for v7 branch to lock update. But isn’t supposed to occurs, if you I already installed logger on your project (lock file, should have v6). The only reason, you have this issue it’s because I have manually updated the logger @SavvasVincentTamosaitis. Right?

Also, @tsed/logger v7 work with @tsed/di v7 for esm. So lock the v7 logger usage for these projects isn’t a good solution, because esm is the future of node.js.

I encourage you to migrate your project on esm and to v7/v8 (v6 is outdated so I won’t release fix for that).
See you

@SavvasVincentTamosaitis
Copy link
Author

My project is not dependent on tsed/logger or tsed/logger-file except through, so it is installing per the semantic versioning rules set in other tsed packages. I have a workaround to override the versioning set by these packages. I think it's likely that, if this issue was only resolved for tsed/di, other tsed packages may cause the same issue.

I have tried migrating to esm to solve this; my migration probably wasn't complete as my attempts did not solve the issue. No problem with not resolving the issue for v6- I added this insight to provide more information.

@Romakita
Copy link
Collaborator

Romakita commented Dec 5, 2024

Hi @SavvasVincentTamosaitis

Your project depend on @tsed/logger if you use @tsed/common.

The CLI add @tsed/logger explicitly on generated project. You haven't used probably the CLI to start your project. I recommend to add @tsed/logger as dependencies with fixed version to resolve the problem ;).

To migrate your project on v7:

To migrate your poject on v8:

Also generate a project using CLI and pick the package.json configuration (scripts, tools, tsconfig) to help you.

I can also help on ESM migration if you are blocked (but you have to take tiers sponsors for that - one time tiers or recurrent- here: https://github.com/sponsors/Romakita)

See you

@csnate
Copy link

csnate commented Dec 12, 2024

I ran into a similar issue with most v7 versions of @tsed/common and found a work around. The peerDependencies of @tsed/common for @tsed/logger is set to >=6.7.5 which matches the v7 version of @tsed/logger which is a ESM module and not a CommonJS module.

If you are still on v7 of TsED and can't upgrade to v8 yet, you can add the following to your package.json:

{
  "overrides": {
    "@tsed/logger": "6.7.5",
    "@tsed/logger-file": "6.7.5"
  }
}

And then re-install @tsed/common. This will peg the tsed/logger version to the CommonJS version. More info on overrides

@SavvasVincentTamosaitis
Copy link
Author

@csnate I haven't tested yet, but it also look like v7.85.1 may have resolved the issue. I see that the semantic versioning has locked down down the logger dependencies to the latest minor version now. See 9b668aa

@Romakita
Copy link
Collaborator

Hi @csnate @SavvasVincentTamosaitis
My apologize for that. I haven’t seen the side effect when I released the logger v7. I know, this is not obvious for you that the @tsed/logger is a dependency of @tsed/di and @tsed/common. In fact, this package is here since the framework exists ;)

I haven’t applied a strict version flag, to let any project to use any version of the tsed logger. It wasn’t an issue until right know, due to the ecosystem migration to ESM.

I finally published to fix, one for v6 and one for v7, to lock the logger version and close this issue ^^.

see you and sorry again for this issue!

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

No branches or pull requests

3 participants