Vacms 16105 metatags #269
Vacms 16105 metatags #269
Conversation
- Updates types for news stories and story listings to include metatag prop. - Updates tests/mock data to accommodate metatag prop.
…ased on environment.
- Adds newly created <Meta> component to [[...slug]].tsx
| id: string | ||
| context?: GetServerSidePropsContext |
There was a problem hiding this comment.
The QueryOpts type definition provided by next-drupal doesn't seem correct to me.
export declare type QueryOpts<Options> = Options & {
context?: GetStaticPathsContext | GetServerSidePropsContext;
};
It seems like it should be context?: GetStaticPropsContext | GetServerSidePropsContext. After all, we are querying data from within getStaticProps and, as such, passing in a GetStaticPropsContext property. So, our query functions need to accept a context that is of type GetStaticPropsContext.
We have a lot of instances of QueryOpts<{some type definition}>, but, for now, I simply removed this from stories and story listings and explicitly added the context property with the type we want (ExpandedStaticPropsContext, which is a decorated form of GetStaticPropsContext). We'll need to address the other instances later.
src/lib/utils/environment.ts
Outdated
| export const generateAbsoluteUrlFromEnv = (relativeUrl: string) => | ||
| generateAbsoluteUrlFromBuildType( | ||
| relativeUrl, | ||
| process.env.NEXT_PUBLIC_BUILD_TYPE |
There was a problem hiding this comment.
i think we may want to use APP_ENV over NEXT_PUBLIC_BUILD_TYPE
There was a problem hiding this comment.
See my other comment. I'm not sure either of these is ultimately what we want. I think this is a bit deeper. The rhetorical question I would ask is, what is the value of APP_ENV on Tugboat?
There was a problem hiding this comment.
it's tugboat, so naively I thought something like:
TUGBOAT: 'tugboat',
[environments.TUGBOAT]: {
hostname: process.env.TUGBOAT_DEFAULT_SERVICE_URL_HOST,
origin: process.env.TUGBOAT_DEFAULT_SERVICE_URL,
},
There was a problem hiding this comment.
This is clarifying. I need to chew on things still, but this helps paint the picture of your vision.
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <link | ||
| rel="canonical" | ||
| href={generateAbsoluteUrlFromEnv(canonicalLink)} |
There was a problem hiding this comment.
I'm not sure that generateAbsoluteUrlFromEnv() is working as intended
Both og:url and the <link rel="canonical" href="..."> seem to be relative only, which returns if the build type is not valid (currently NEXT_PUBLIC_BUILD_TYPE)
There was a problem hiding this comment.
This relates to the discussion in Slack.
This will not work right now. We need to do some work to nail down what different ENV vars mean, and how we are going to use them.
This was an initial pass, but I'm now thinking we should not be using NEXT_PUBLIC_BUILD_TYPE or NEXT_PHASE or any generic environment identifier to generate absolute URLs. A dev vs. prod build says very little about where the build will ultimately live. We can do a production build locally, and we should still be able to generate local URLs.
There was a problem hiding this comment.
yes, completely agree
I think ultimately we will need some separate options handling from the .env files, something like: department-of-veterans-affairs/va.gov-cms#15322 (analogous to the cli options available to content build https://github.com/department-of-veterans-affairs/content-build/blob/main/src/site/stages/build/options.js )
…URLs. This needs more consideration going forward, but it is good for now.
ryguyk
force-pushed
the
VACMS-16105-metatags
branch
from
c7a94f9 to
2567b6a
Compare
Description
Relates to department-of-veterans-affairs/va.gov-cms#16105
Custom meta tags were made available via JSON:API in recent work. This PR places those custom meta tags, along with other global meta tags, on the page.
Testing done
Manual; see QA steps
QA steps