Skip to content

refactor: env check with function #21310

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

Merged
merged 1 commit into from
Jun 21, 2025
Merged

refactor: env check with function #21310

merged 1 commit into from
Jun 21, 2025

Conversation

crazywoola
Copy link
Member

Summary

This pull request refactors the configuration logic in web/config/index.ts to improve readability and maintainability. The changes replace repetitive code with a reusable helper function for determining boolean configuration values.

Refactoring for improved maintainability:

  • Introduced a helper function getBooleanConfig to centralize the logic for determining boolean configuration values based on environment variables or DOM attributes.
  • Replaced individual configuration logic for ENABLE_WEBSITE_JINAREADER, ENABLE_WEBSITE_FIRECRAWL, and ENABLE_WEBSITE_WATERCRAWL with calls to the new getBooleanConfig function.
  • Simplified default values for enableWebsiteJinaReader, enableWebsiteFireCrawl, and enableWebsiteWaterCrawl by initializing them explicitly before applying configuration logic.

Screenshots

const getBooleanConfig = (envVar: string | undefined, attr: string) => {
  if (envVar !== undefined && envVar !== '')
    return envVar === 'true'
  const attrValue = globalThis.document?.body?.getAttribute(attr)
  if (attrValue !== undefined && attrValue !== '')
    return attrValue === 'true'
  return false
}

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@crazywoola crazywoola requested a review from Copilot June 21, 2025 03:13
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. ☕️ typescript labels Jun 21, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the configuration logic to improve readability and maintainability by introducing a helper function for determining boolean configuration values from either environment variables or DOM attributes.

  • Added a reusable helper function, getBooleanConfig.
  • Replaced repetitive config checks with calls to getBooleanConfig.
  • Initialized configuration variables with explicit default values before applying the new logic.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 21, 2025
@crazywoola crazywoola merged commit d333aac into main Jun 21, 2025
9 checks passed
@crazywoola crazywoola deleted the feat/update-env-check branch June 21, 2025 10:33
@kurokobo
Copy link
Contributor

kurokobo commented Jun 21, 2025

@crazywoola
I was just about to edit this file.
getBooleanConfig always returns false when neither an environment variable nor an attribute is present, so I don't think there's any point in initializing with a default value using let. In other words, the default will end up being false regardless of the initial value.

Wouldn't it be better to pass the default value as the third argument to getBooleanConfig, and have the last return use that default?

diff --git a/web/config/index.ts b/web/config/index.ts
index e8aa404be..b991cde49 100644
--- a/web/config/index.ts
+++ b/web/config/index.ts
@@ -304,22 +304,15 @@ else if (globalThis.document?.body?.getAttribute('data-public-max-iterations-num

 export const MAX_ITERATIONS_NUM = maxIterationsNum

-let enableWebsiteJinaReader = true
-let enableWebsiteFireCrawl = true
-let enableWebsiteWaterCrawl = false
-
-const getBooleanConfig = (envVar: string | undefined, attr: string) => {
+const getBooleanConfig = (envVar: string | undefined, attr: string, defaultValue: boolean) => {
   if (envVar !== undefined && envVar !== '')
     return envVar === 'true'
   const attrValue = globalThis.document?.body?.getAttribute(attr)
   if (attrValue !== undefined && attrValue !== '')
     return attrValue === 'true'
-  return false
+  return defaultValue
 }

-enableWebsiteJinaReader = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_JINAREADER, 'data-public-enable-website-jinareader')
-enableWebsiteFireCrawl = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_FIRECRAWL, 'data-public-enable-website-firecrawl')
-enableWebsiteWaterCrawl = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_WATERCRAWL, 'data-public-enable-website-watercrawl')
-export const ENABLE_WEBSITE_JINAREADER = enableWebsiteJinaReader
-export const ENABLE_WEBSITE_FIRECRAWL = enableWebsiteFireCrawl
-export const ENABLE_WEBSITE_WATERCRAWL = enableWebsiteWaterCrawl
+export const ENABLE_WEBSITE_JINAREADER = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_JINAREADER, 'data-public-enable-website-jinareader', true)
+export const ENABLE_WEBSITE_FIRECRAWL = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_FIRECRAWL, 'data-public-enable-website-firecrawl', true)
+export const ENABLE_WEBSITE_WATERCRAWL = getBooleanConfig(process.env.NEXT_PUBLIC_ENABLE_WEBSITE_WATERCRAWL, 'data-public-enable-website-watercrawl', false)

jsincorporated pushed a commit to jsincorporated/asaAi that referenced this pull request Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants