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

Allow to pass .ts files as config with Bun when ts-node is disabled #4701

Closed
octet-stream opened this issue Sep 15, 2023 · 14 comments · Fixed by #4702
Closed

Allow to pass .ts files as config with Bun when ts-node is disabled #4701

octet-stream opened this issue Sep 15, 2023 · 14 comments · Fixed by #4702
Labels
enhancement New feature or request

Comments

@octet-stream
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Hi, I'm experimenting with MikroORM and Bun, and I find out that MikroORM's configuration loader ignores .ts files when ts-node is not enabled. And while this constraint is suitable for node, it is not necessary for Bun since it support TS files out of the box.

Describe the solution you'd like

I think it would be nice to add at least an option for configurator loader to pass .ts files when this option enabled. Something like bypassTsFiles flag in mikro-orm section of package.json. When enabled, this filter will be ignored same was as if ts-node was enabled.

Describe alternatives you've considered

Enabling ts-node works with Bun 1.0.1, but this running code from ts-node with Bun is not necessary.

Additional context

No additional context. But I think I can send a PR upon agreement on implementation details.

@octet-stream octet-stream added the enhancement New feature or request label Sep 15, 2023
@B4nan
Copy link
Member

B4nan commented Sep 16, 2023

Makes sense, PR welcome. Maybe the name should be a bit better, as its not really about bypassing the TS files? Ideally we should have a guide about bun in the docs.

@octet-stream
Copy link
Contributor Author

octet-stream commented Sep 16, 2023

Maybe the name should be a bit better, as its not really about bypassing the TS files?

Yes, this flag should enable ts files without ts-node, so maybe something like allowTs? Or alwaysAllowTs?

Actually, I have another thought: Maybe allow ts files by default, but then if we fail to import config with .ts extension, then just suggest ts-node?

Something like (just a pseudo code)

const path = getConfigPath()
try {
  await import(path)
} catch (e) {
  // Check for error type, and then:
  if (path.endsWith(".ts")) {
    throw new Error("Failed attempt to load ORM config from .ts file. Try enabling ts-node with useTsNode flag in package.json")
  }
}

Something like this.

Ideally we should have a guide about bun in the docs.

I think we need to have mikro-orm tests to pass with bun first. I haven't tested this yet.

@octet-stream
Copy link
Contributor Author

This also should work with entities, subscribers, migrations and other stuff that might be loaded by MikroORM itself.

@B4nan
Copy link
Member

B4nan commented Sep 17, 2023

I would love to get rid of the useTsNode flag, but it cant be at the cost of some errors/warning for js only projects.

B4nan pushed a commit that referenced this issue Sep 17, 2023
…e` (#4702)

### Details

This PR introduces two new options to allow `.ts` files as configs when
`ts-node` is not enabled:

- `alwaysAllowTs` flag in `mikro-orm` section of `package.json`
- `MIKRO_ORM_CLI_ALWAYS_ALLOW_TS` environment variable

### Changes

- [x] Add optional `alwaysAllowTs` field on `Settings` interface in
`ConfigurationLoader`;
- [x] Return `.ts` configs files from
`ConfigurationLoader.getConfigPaths()` when `alwaysAllowTs` option is
enabled;
- [x] Add tests;
- [x] Add documentation for these new options

Fixes #4701
@octet-stream
Copy link
Contributor Author

octet-stream commented Sep 17, 2023

I would love to get rid of the useTsNode flag, but it cant be at the cost of some errors/warning for js only projects.

It's ok, but I would rather report errors to tell a user they're doing something wrong than silently ignoring .ts files altogether.

@B4nan
Copy link
Member

B4nan commented Sep 17, 2023

Well, but it's completely fine to not use TS in your project, and that needs to be the baseline, TS should be opt in, not opt out.

@octet-stream
Copy link
Contributor Author

Makes sense, then I think default .js configs should've come first in the list, so that MikroORM will try to read those first (after custom paths).

@octet-stream
Copy link
Contributor Author

So, the algorithm would be: custom paths -> default .js -> default .ts, if none found - then show an error.
If somebody specify TS file as config, they probably know what they're doing, and if such file exists, but it can't be executed by runtime (can't be imported due to unsupported file format) - the error should be shown. But that's just my thoughts :)

@B4nan
Copy link
Member

B4nan commented Sep 17, 2023

That would be heavily breaking, as it would mean a compiled js file has precedence over the ts source file.

We would need to have a way for users to say the project is TS, I really think the defaults need to stay working as they do now.

@narcopata
Copy link

Sorry, is this really already fixed? I tried to run the CLI on my Bun MikroORM API, had setup the config .ts file, set the alwaysAllowTs flag on "mikro-orm" as true on package.json, and still I have the error: - configuration not found (Unknown file extension ".ts" for /home/rws/projects/empresa_alberto_testes/test-back/src/database/mikro-orm.config.ts).

@octet-stream
Copy link
Contributor Author

Could you provide minimal reproduction repo, please?

@narcopata
Copy link

@octet-stream
Copy link
Contributor Author

You need to run command with --bun flag in order to run Mikro ORM CLI via Bun instead of Node.js. Like this: bun run --bun mikro-orm debug or bunx --bun mikro-orm debug.

See documentation: https://bun.sh/docs/cli/bunx#shebangs

@narcopata
Copy link

You need to run command with --bun flag in order to run Mikro ORM CLI via Bun instead of Node.js. Like this: bun run --bun mikro-orm debug or bunx --bun mikro-orm debug.

See documentation: https://bun.sh/docs/cli/bunx#shebangs

Thanks it worked!

I think it would be interesting to have this usage example available on
MikroORM's documentation (along with npm, yarn, npx, etc.).

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

Successfully merging a pull request may close this issue.

3 participants