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

refactor(core): Move backend config to a separate package #9325

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

netroy
Copy link
Member

@netroy netroy commented May 7, 2024

This PR moves the config related code into a separate @n8n/config package so that we can use the config on all backend packages (like core and nodes-base) without creating a circular dependency with cli.
This PR also changes the config object to be

  1. an injectable class, that can be used with regular DI code
  2. type-safe without the need for the complicated inference code in packages/cli/node_modules/@types/convict/index.d.ts
  3. annotated with JSDocs on every config property, which adds helpful tooltips in our editors
    image

Every sub-section in the config is also now separately injectable, which we can use to create config objects specific to certain sections of the codebase, instead of using this one global config everywhere.

TODO

  • Move the rest of the config properties into the new package
  • Read values from files for env variables with _FILE suffix
  • Generate a JSON file when building @n8n/config, include it in the published package, and then use this JSON to generate the docs for env variables.
  • Merge InstanceSettings into this package
  • Support reading all properties from .n8n/config

Review / Merge checklist

  • PR title and summary are descriptive
  • Docs updated or follow-up ticket created.
  • Tests included

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels May 7, 2024
packages/@n8n/config/src/configs/database.ts Outdated Show resolved Hide resolved
packages/@n8n/config/src/configs/database.ts Outdated Show resolved Hide resolved
class LoggingConfig {
/** Typeorm logging enabled flag. @default false */
@FromEnv('DB_LOGGING_ENABLED')
readonly enabled: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally there should be a way to have the effect of readonly but without overly narrowing the type and without having to specify it... Not sure if possible but I'll look if there's a way.

packages/@n8n/config/src/configs/database.ts Outdated Show resolved Hide resolved
packages/@n8n/config/.eslintrc.js Outdated Show resolved Hide resolved
packages/@n8n/config/src/decorators.ts Outdated Show resolved Hide resolved
packages/@n8n/config/src/index.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants