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

add dotenv support to PlatformConfigProvider #3743

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

sukovanej
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Oct 7, 2024

🦋 Changeset detected

Latest commit: cc368b4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 25 packages
Name Type
@effect/platform Patch
@effect/cli Patch
@effect/cluster-node Patch
@effect/cluster-workflow Patch
@effect/cluster Patch
@effect/experimental Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/rpc-http Patch
@effect/rpc Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-node Patch
@effect/sql Patch
@effect/cluster-browser Patch
@effect/sql-drizzle Patch
@effect/sql-kysely Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot changed the base branch from main to next-minor October 7, 2024 18:07
@datner
Copy link
Member

datner commented Oct 7, 2024

The name might carry association to some behaviors dotenv has (like variable expansion), theres also dotenvx with its new mutli-env and encryption capabilities. I don't think this blocks the PR in any way, but we should think of the future plan with this. It might be better to just recommend people use dotenvx directly since from there the regular ConfigProvider would just work

@sukovanej
Copy link
Contributor Author

sukovanej commented Oct 7, 2024

The name might carry association to some behaviors dotenv has (like variable expansion)

this PR is somewhat a copy of dotenv and dotenv-expand codebases, so the variable expansion support is included

theres also dotenvx with its new mutli-env and encryption capabilities

I haven't used dotenvx myself, if by multi-env you mean providing different .env.<suffix> file, then that's also supported (the dotenv filename is an input of all the new functions); encryption is not, tho I guess it should be possible to implement a combinator on ConfigProvider dealing with encryption independently of the concrete ConfigProvider instance

It might be better to just recommend people use dotenvx directly since from there the regular ConfigProvider would just work

I think both an in-code API and CLI are valid options, the advantage of an in-code approach is that I can more easily do whatever I want in terms of configuring the application, and with the proposed change it would be a matter of combining layers / effects. At the end of the day it's a question of whether I want to put the responsibility into the typescript code or the shell code, and there is unlikely a definite answer to that, but I prefer the part of my codebase where a type-checker and linter look over my shoulder.

.changeset/neat-olives-learn.md Outdated Show resolved Hide resolved
packages/platform/src/internal/platformConfigProvider.ts Outdated Show resolved Hide resolved
@sukovanej sukovanej changed the title draft: add dotenv support to PlatformConfigProvider add dotenv support to PlatformConfigProvider Oct 7, 2024
@tim-smart tim-smart changed the base branch from next-minor to main October 7, 2024 23:49
@jessekelly881
Copy link
Contributor

jessekelly881 commented Oct 8, 2024

I would just put

require('@dotenvx/dotenvx').config()

at the root of your program and call it a day. It works the same as prefixing the cli call with dotenvx run -- . I don't really see the benefit of trying to copy the functionality of this.

@tim-smart tim-smart merged commit b75ac5d into Effect-TS:main Oct 8, 2024
11 checks passed
@github-actions github-actions bot mentioned this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants