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

Remove env magic #12

Open
3y3 opened this issue Feb 17, 2023 · 1 comment
Open

Remove env magic #12

3y3 opened this issue Feb 17, 2023 · 1 comment

Comments

@3y3
Copy link

3y3 commented Feb 17, 2023

At current time env is accessible everywhere in project (but used only in main module).

Proposal:

  • Disallow env in eslint rules.
  • Add 'resolve-options' helper (allow env here).
  • Add to options all missed entities. For example const appInstallation = process.env.APP_INSTALLATION; can not be configured without env.
  • Reverse default behavior for disableDotEnv
  • Make more options accessible from env

Proposal+:

Looks like options.config is a flat object.
Maybe it should be top level param

constructor(options: InitOptions = {}) {}

constructor(options: InitOptions & AppConfig = {}) {}

So most part of configuration will be accessible from env

# appTracingEnabled
APP_TRACING_ENABLED=false

But I understand problems with appSensitiveKeys in this keys.

Most interested api looks like:

import {NodeKit, configure} from 'nodekit';

const nodekit = new NodeKit(configure({
  env: true,
  file: require.resolve('./config')
}));
@resure
Copy link
Contributor

resure commented Feb 20, 2023

First part of proposal sounds good, except for this:

Reverse default behavior for disableDotEnv

I understand where it's coming from, but in real-life usage that would be an option that almost always would be passed with true value. Maybe we should think about some kind of different NodeKit modes:

  1. One without any side-effects (including dotenv, shutdown handlers, etc.)
  2. Another for applications, with side-effects enabled

Proposal+:
Looks like options.config is a flat object

It's not, it can contain any type of values, including nested objects.

But I understand problems with appSensitiveKeys in this keys.

Yeah, that's too

@resure resure moved this to 🗃️ Later in NodeKit Development Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🗃️ Later
Development

No branches or pull requests

2 participants