Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

feat(dns): add option to enable app-level DNS caching #727

Merged
merged 5 commits into from
May 4, 2022

Conversation

10xLaCroixDrinker
Copy link
Member

@10xLaCroixDrinker 10xLaCroixDrinker commented Apr 29, 2022

Description

Add option in appConfig to enable app-level DNS caching via cacheable-lookup

Motivation and Context

Perf enhancement

How Has This Been Tested?

Implemented in a module.

Unit tests

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update
  • Security update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • Performance tests should be ran against the server prior to merging.
  • This change impacts caching for client browsers.
  • This change impacts HTTP headers.
  • This change adds additional environment variable requirements for One App users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using One App?

@github-actions
Copy link
Contributor

github-actions bot commented Apr 29, 2022

📊 Bundle Size Report

file name size on disk gzip
app.js 314.7KB 93.4KB
runtime.js 15.6KB 5.5KB
vendors.js 148.2KB 47.6KB
app~vendors.js 431.2KB 112.6KB
service-worker-client.js 17.1KB 5.3KB
legacy/app.js 335.6KB 98.3KB
legacy/runtime.js 15.6KB 5.5KB
legacy/vendors.js 211KB 59.7KB
legacy/app~vendors.js 453KB 118.3KB
legacy/service-worker-client.js 17.6KB 5.5KB

Generated by 🚫 dangerJS against 33e9e30

bishnubista
bishnubista previously approved these changes Apr 29, 2022
@10xLaCroixDrinker 10xLaCroixDrinker requested a review from a team April 29, 2022 22:08
@Matthew-Mallimo Matthew-Mallimo requested a review from a team May 2, 2022 16:25
Co-authored-by: Matthew Mallimo <matthew.c.mallimo@aexp.com>
Matthew-Mallimo
Matthew-Mallimo previously approved these changes May 3, 2022
@Matthew-Mallimo Matthew-Mallimo requested a review from a team May 3, 2022 14:54
Copy link
Contributor

@JAdshead JAdshead left a comment

Choose a reason for hiding this comment

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

just a little more info in docs

docs/api/modules/App-Configuration.md Outdated Show resolved Hide resolved
uninstallCacheableLookup();
} else if (
enabled === true
&& (cacheableInstance == null || cacheableInstance.maxTtl !== maxTtl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: Jonny Adshead <JAdshead@users.noreply.github.com>
@10xLaCroixDrinker 10xLaCroixDrinker requested a review from a team May 4, 2022 16:27
@10xLaCroixDrinker 10xLaCroixDrinker enabled auto-merge (squash) May 4, 2022 17:06
@10xLaCroixDrinker 10xLaCroixDrinker merged commit 69dff8d into main May 4, 2022
@10xLaCroixDrinker 10xLaCroixDrinker deleted the feature/cacheable-lookup branch May 4, 2022 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants