Feature/typescript project #773
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: b66776f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
1 similar comment
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
| @@ -1,9 +1,13 @@ | |||
| import { BlocksRenderer, YoutubeLiteBlock, ImageBlock } from '@headstartwp/core/react'; | |||
| import { TwitterBlock, ImageComponent, LinkBlock } from '@headstartwp/next'; | |||
| import { FC } from 'react'; | |||
There was a problem hiding this comment.
question: why are we removing FC and therefore the return type?
There was a problem hiding this comment.
FC is confusing, at some point it had children but in recent versions oof React types it doesnt.
I also don't like PropsWithChildren bc it makes children optional and sometimes we want to it be required. So for the most part, just typing the props is enough and actually allow us to better describe the component intention
There was a problem hiding this comment.
and this one doesn't need children at all so IMO best to just type the props
There was a problem hiding this comment.
Thanks for the explanation. I prefer using function anyway but usually use FC when defining a component as a const since it adds the return type. With function I'd usually add the return type ReactElement, etc
I like props with children because it uses ReactNode but I agree with you if you want to be more strict with requiring children then it is not a good choice.
There was a problem hiding this comment.
I've added a few comments throughout code, mostly as questions/clarifications :)
Love the amount of satisfies usage ❤️
A note on usage of interfaces. Is there a particular reason why you opt for using interfaces over types?
Although nowadays there's barely any differences between types & interfaces, there's a couple of things worth considering (I'm sure you're aware of these already):
- Declaration Merging - ensure that this may not produce unwanted side-effects, unless you explicitly do want to use it.
- Types are generally a bit more flexible, and can provide slightly cleaner syntax when using utility types and
typeoffor extracting types, using type aliases & unions, etc.
We generally tend to prefer using types, and stick to interfaces in specific situations:
- when using with a class, for example if we write SOLID compliant code - our classes and sometimes also function parameters will use interfaces over types;
- in scenarios where we know in advance that there will be a case of extending the base interface with additional properties, especially if it is exposed outside of its own project/library.
All in all, from our experience, types seem to provide slightly nicer DX and cleaner syntax, and interfaces we tend to leave for specific use-cases.
| ]); | ||
|
|
||
| return addHookData(settledPromises, { revalidate: 60 }); | ||
| } catch (e) { | ||
| return handleError(e, context); | ||
| } | ||
| }; | ||
| }) satisfies HeadlessGetStaticProps; |
| export interface AddHokoDataBaseProps { | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
You may want to be careful with allowing wildcard props, unless you're explicitly planning to access them.
There was a problem hiding this comment.
addHookData should be able to receive props that would then be passed to the page component in Next.js. This can be arbitrary props. This was mostly to allow TS to accept arbritary props and to infer the type from the passed props as well.
projects/wp-nextjs/tsconfig.json
Outdated
| "module": "esnext", | ||
| "moduleResolution": "node16", | ||
| "module": "ESNext", | ||
| "moduleResolution": "node", | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, | ||
| "jsx": "preserve" |
There was a problem hiding this comment.
A couple additions that may be helpful, but not necessarily add too much strictness to the code. More around good coding practices rather than TS as such.
noFallthroughCasesInSwitch: true- Definition - ensures you are not skipping anything in a switch statement, and always at least provide adefaultoption.noImplicitAny- Definition - ensures you define asanyexplicitly, rather than implicitly assume it. This may add a bunch more changes to this PR however, but worth checking.strictBindCallApply- Definition - can be helpful.strictNullChecks- Definition - as the name implies, stricter null checks.noUnusedParameters- Definition - also as the name implies, does not allow unused parameters to remain in code.
There was a problem hiding this comment.
I think it would be nice to have a tsconfig.strict.json and tsconfig.json. The code in this repo would pass the strict tsconfig. Projects can choose whether they want to enforce strict TS or not depending on the engineers on the project.
There was a problem hiding this comment.
noFallthroughCasesInSwitch and noUnusedParameters have very similar eslint rules in the 10up eslint config so I see no harm in adding it to TS. strictBindCallApply I think it's helpful and we don't write code using bind, .call very often so don't think it will make things too strict most of the time and strictNullChecks can help catch bugs so I think it makes sense.
noImplicitAny is the most strick one IMO but based on our convo we do want any to be a escape hatch for unexperience engineers so I'm fine forcing them to explicitly set any when they do need the escape hatch which would also make it easier to come back and update types later.
I don't think two configs would make sense as it would just add more confusion IMO.
|
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
📦 Next.js Bundle Analysis for @10up/headless_frameworkThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
Description of the Change
Removes the old TS projects and makes the default project (wp-nextjs) TypeScript based.
How to test the Change
Simply browse through the vercel preview url and ensure everything still works as expected
Checklist: