feat(RemoteContentLoader): navDataPrefix option #583
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 445d4fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Ah I see now. So generally it's |
| @@ -95,22 +104,28 @@ export default class RemoteContentLoader implements DataLoader { | |||
| if (!this.opts.mainBranch) this.opts.mainBranch = 'main' | |||
| if (!this.opts.scope) this.opts.scope = {} | |||
| if (!this.opts.remarkPlugins) this.opts.remarkPlugins = [] | |||
|
|
|||
| this.navDataPrefix = this.opts.navDataPrefix || this.opts.basePath | |||
There was a problem hiding this comment.
suggestion: convention is for everything to be read from this.opts, so we can use a similar default approach above and assign to this.opts.basePath if it is unset. No need for the extra class property
There was a problem hiding this comment.
Good call! I'll do that
There was a problem hiding this comment.
Removed the extra property: 445d4fb
Yea, this has been one of those assumptions that hasn't caused issues in the past, but not sort of does |
thiskevinwang
added
the
release:canary
📦 Canary Packages PublishedLatest commit: 445d4fb Published 1 packages@hashicorp/react-docs-page@16.1.0-canary-20220505214234 |
Description
This adds a
navDataPrefixoption to ourRemoteContentLoader.This helps to address a previous assumption that
basePathwould always be the same fordocandnav-datalookups from our API, which is not necessarily going to be the case going forward.🎟️ Asana Task
🔍 Preview Link
Description
Describe this pull request: what is it aiming to achieve? What should someone reviewing this PR know? If this PR includes visual changes, consider including before / after screenshots, or an animated gif showing interactions.
PR Checklist 🚀
Items in this checklist may not may not apply to your PR, but please consider each item carefully.