Skip to content

🐛 Always ignore existing root resources during asset discovery#390

Merged
wwilsman merged 1 commit intomasterfrom
ww/fix-duplicate-root-resources-when-discovery-cache-is-disabled
Jun 23, 2021
Merged

🐛 Always ignore existing root resources during asset discovery#390
wwilsman merged 1 commit intomasterfrom
ww/fix-duplicate-root-resources-when-discovery-cache-is-disabled

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

In asset discovery when the cache is disabled, it doesn't matter if the getResource option returns an existing resource. The existing resource will always be overridden with new response. However, root resources aren't normally included with other resources. So when a root resource requests itself (such as a canonical link) and cache is disabled, a new resource is created for the root response (but is-root is not set since it happens during normal discovery). Later, when the root resource is added to the discovered resources, it ends up in the list of resources twice.

Fixing this is pretty straight forward. We just need to always skip capturing the root resource, even if the cache is disabled.

Fixes #388

@wwilsman wwilsman added the 🐛 bug Something isn't working label Jun 23, 2021
@wwilsman wwilsman requested a review from Robdel12 June 23, 2021 20:45
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 Solid find after some dog pile debugging 😆

@wwilsman wwilsman enabled auto-merge (squash) June 23, 2021 20:53
@wwilsman wwilsman merged commit 7cdc108 into master Jun 23, 2021
@wwilsman wwilsman deleted the ww/fix-duplicate-root-resources-when-discovery-cache-is-disabled branch June 23, 2021 20:53
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [eslint](https://github.com/eslint/eslint) from 7.31.0 to 7.32.0.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md)
- [Commits](eslint/eslint@v7.31.0...v7.32.0)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errors attempting to upload multiple resources with the same resource url

2 participants