fix for reading custom env files (for e.g. ormconfig.env) #3361

pdeszynski · 2019-01-04T13:39:27Z

Added missing extension when reading <custom_name>.env file

Tested on TypeORM v0.2.11

Addes missing extension when reading .env file

pleerock · 2019-01-04T16:33:15Z

Can you please tell what issue you have? Because we have changed this area recently. Do you have something broken or what? adding @steevsachs

pdeszynski · 2019-01-04T18:19:57Z

@pleerock for me it stopped to read ‘ormconfig.env’ file as it was trying to load file without any extension.

vlapo · 2019-01-04T18:36:15Z

I think we should finally fix and unskip tests for this area. I know there is problem with config files not accesible during tests. Maybe using ts-node in tests will fix this after #3332 merged?

pleerock · 2019-01-07T11:43:02Z

@vlapo yes, it will.

I'm waiting for @steevsachs review of this PR.

steevsachs · 2019-01-07T11:56:20Z

No problem, I will review this morning.

steevsachs · 2019-01-07T14:36:36Z

I can't reproduce the problem described here, can anyone provide more details in case I'm overlooking something?

this.baseFilePath in a ConnectionOptionsReader is the full path of the config file, so appending .env to that file name will always result in a different file path from the config file path.

This will also break functionality for a plain .env file because foundFileFormat will equal env but the ConnectionOptionsReader would try to load .env.env.

If you manually move the config file mocks into the appropriate build dir and unskip the last test in test/functional/connection-options-reader/connection-options-reader.ts, the tests will fail.

pdeszynski · 2019-01-07T14:39:31Z

@steevsachs when I was testing this on 0.2.11 and having ormconfig.env file it was trying to load in my case ormconfig file instead (without any extension). After this change I was also testing it with .env file and it was still correctly loaded.

But maybe it's already fixed in master. I will try to prepare reproducible case in a separate repository tonight.

pdeszynski · 2019-01-07T19:33:13Z

@steevsachs Minimal reproducible code https://github.com/pdeszynski/typeorm-config-failing

yarn install
yarn start

It should end up with

(node:36943) UnhandledPromiseRejectionWarning: Error: No connection options were found in any of configurations file.
    at ConnectionOptionsReader.<anonymous> (/Users/piteer/workspace/typeorm-config/src/connection/ConnectionOptionsReader.ts:41:19)
    at step (/Users/piteer/workspace/typeorm-config/node_modules/typeorm/connection/ConnectionOptionsReader.js:32:23)
    at Object.next (/Users/piteer/workspace/typeorm-config/node_modules/typeorm/connection/ConnectionOptionsReader.js:13:53)
    at fulfilled (/Users/piteer/workspace/typeorm-config/node_modules/typeorm/connection/ConnectionOptionsReader.js:4:58)
    at process._tickCallback (internal/process/next_tick.js:68:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:744:11)
    at Object.<anonymous> (/Users/piteer/workspace/typeorm-config/node_modules/ts-node/src/bin.ts:157:12)
    at Module._compile (internal/modules/cjs/loader.js:688:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
    at Module.load (internal/modules/cjs/loader.js:598:32)

Renaming ormconfig.env to .env will solve this problem.

The same problem occurs when running this code against typeorm@0.2.12-rc.1

steevsachs · 2019-01-07T20:24:41Z

Thank you for that, @pdeszynski. So the issue here is specifically support for ormconfig.env, which occurs because we default the base config name to ormconfig and correctly detect a .env config type, but then don't by default load from ormconfig.env.

We can't just append .env because it would break support for any environment type config that has specified .env in its filename.

steevsachs · 2019-01-07T20:25:37Z

src/connection/ConnectionOptionsReader.ts

@@ -94,7 +94,7 @@ export class ConnectionOptionsReader {
        // if .env file found then load all its variables into process.env using dotenv package
        if (foundFileFormat === "env") {
            const dotenv = PlatformTools.load("dotenv");
-            dotenv.config({ path: this.baseFilePath });
+            dotenv.config({ path: `${this.baseFilePath}.env` });


The extension should only be appended if possibleExtension is not equal to .env.

@steevsachs you're right, but this was not falling for this foundFileFormat === "env" condition, if i had .env file instead of ormconfig.env What about the else case at https://github.com/typeorm/typeorm/pull/3361/files/d0e8fa3af49052cd13703f27f7c31e2d6e5d9d5e#diff-fd8d31a6463f188113f6a9f43f77612fR98 ? Isn't it solving .env file case? When testing after applying this patch I had both cases working correctly (but I didn't run functional tests).

The else condition only fires if the config file format is not .env, ie ormconfig.json. This ensure environment is still loaded in those cases.

I believe the reason .env is working is that

ConnectionOptionsReader uses ormconfig as its base file name...

foundFileFormat will equal undefined at line 95 because there is no ormconfig.* at the base file path

Thus the else condition fires and loads the .env file.

nimish · 2019-07-18T20:28:46Z

Hi all, this is still broken. Will someone merge this to fix this defect in the docs and the config loader? The only useful workaround is to switch to .env.

Ivanca · 2019-07-31T20:06:11Z

Can't believe this fix hasn't been accepted, right now using ormconfig.env or foo.env or bar.env doesnt work at all, the only workaround is to name the file as ".env" cc @steevsachs

karfau · 2019-12-08T20:03:01Z

Here is my workaround to deal with the dropped support for loading ormconfig.env by default that works for typeorm CLI and without changing any existing code:
https://gist.github.com/karfau/b6d0927628f6662ca6d892153562522f

This allowed me to upgrade from v0.2.7 to v0.2.21 without any (other) issues.

imnotjames

This needs a test to confirm that it actually does what it's intended to. Can you add at least one that confirms the behavior?

imnotjames · 2020-10-09T03:29:10Z

Also there's a conflict.

steevsachs · 2020-10-09T13:06:38Z

This PR seems way out of sync, but I'm pretty sure a viable fix for master is

if (PlatformTools.fileExist(this.baseFilePath)) {
   PlatformTools.dotenv(this.baseFilePath);
} else if (PlatformTools.fileExist(`${this.baseFilePath}.env`)) {
   PlatformTools.dotenv(`${this.baseFilePath}.env`);
}

on line 98 of ConnectionOptionsReader.

There's definitely other ways to handle this, but I think this one will restore compat without breaking anything.

I don't know that I'll have time to refamiliarize myself with this repo enough to get testing working in the short term, so anyone else please feel free to run with this! If no one does, I'll try to make some time in the next couple weeks.

imnotjames · 2020-10-16T08:44:21Z

Actually - I don't think this is broken? I've created some tests and cannot replicate the issue. Attempting via the repo given.

pdeszynski · 2020-10-16T08:47:09Z

Hello all, I've completely forgot about this PR - if you still need it I can take care of it, maybe during this weekend

imnotjames · 2020-10-16T08:50:03Z

Ok - in the repo I can replicate it but not in our tests - weird! In both cases I am using ts-node.

EDIT: No wonder it was working, I was running the wrong test!!

imnotjames · 2020-10-16T09:03:45Z

Hello all, I've completely forgot about this PR - if you still need it I can take care of it, maybe during this weekend

If you'd like! I'll let you know if I end up getting anywhere today on it. :)

imnotjames · 2020-10-16T17:45:31Z

@pdeszynski I think I've got it fixed in #6922

fix for reading custom env files (for e.g. ormconfig.env)

d0e8fa3

Addes missing extension when reading .env file

steevsachs reviewed Jan 7, 2019

View reviewed changes

imnotjames added the needs tests label Oct 7, 2020

imnotjames self-requested a review October 7, 2020 15:51

imnotjames requested changes Oct 9, 2020

View reviewed changes

imnotjames self-assigned this Oct 9, 2020

imnotjames mentioned this pull request Oct 16, 2020

fix: correct reading of custom ormconfig.env files #6922

Merged

imnotjames closed this in #6922 Oct 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for reading custom env files (for e.g. ormconfig.env) #3361

fix for reading custom env files (for e.g. ormconfig.env) #3361

pdeszynski commented Jan 4, 2019 •

edited

pleerock commented Jan 4, 2019

pdeszynski commented Jan 4, 2019

vlapo commented Jan 4, 2019

pleerock commented Jan 7, 2019

steevsachs commented Jan 7, 2019

steevsachs commented Jan 7, 2019

pdeszynski commented Jan 7, 2019 •

edited

pdeszynski commented Jan 7, 2019

steevsachs commented Jan 7, 2019

steevsachs Jan 7, 2019

pdeszynski Jan 7, 2019 •

edited

steevsachs Jan 7, 2019

nimish commented Jul 18, 2019

Ivanca commented Jul 31, 2019 •

edited

karfau commented Dec 8, 2019

imnotjames left a comment

imnotjames commented Oct 9, 2020

steevsachs commented Oct 9, 2020

imnotjames commented Oct 16, 2020 •

edited

pdeszynski commented Oct 16, 2020

imnotjames commented Oct 16, 2020 •

edited

imnotjames commented Oct 16, 2020

imnotjames commented Oct 16, 2020

fix for reading custom env files (for e.g. ormconfig.env) #3361

fix for reading custom env files (for e.g. ormconfig.env) #3361

Conversation

pdeszynski commented Jan 4, 2019 • edited

pleerock commented Jan 4, 2019

pdeszynski commented Jan 4, 2019

vlapo commented Jan 4, 2019

pleerock commented Jan 7, 2019

steevsachs commented Jan 7, 2019

steevsachs commented Jan 7, 2019

pdeszynski commented Jan 7, 2019 • edited

pdeszynski commented Jan 7, 2019

steevsachs commented Jan 7, 2019

steevsachs Jan 7, 2019

Choose a reason for hiding this comment

pdeszynski Jan 7, 2019 • edited

Choose a reason for hiding this comment

steevsachs Jan 7, 2019

Choose a reason for hiding this comment

nimish commented Jul 18, 2019

Ivanca commented Jul 31, 2019 • edited

karfau commented Dec 8, 2019

imnotjames left a comment

Choose a reason for hiding this comment

imnotjames commented Oct 9, 2020

steevsachs commented Oct 9, 2020

imnotjames commented Oct 16, 2020 • edited

pdeszynski commented Oct 16, 2020

imnotjames commented Oct 16, 2020 • edited

imnotjames commented Oct 16, 2020

imnotjames commented Oct 16, 2020

pdeszynski commented Jan 4, 2019 •

edited

pdeszynski commented Jan 7, 2019 •

edited

pdeszynski Jan 7, 2019 •

edited

Ivanca commented Jul 31, 2019 •

edited

imnotjames commented Oct 16, 2020 •

edited

imnotjames commented Oct 16, 2020 •

edited