Migrate pages from Page.getInitialProps to getServerSideProps

[edmundito]

Change description

This updates all UI pages to use getServerSideProps over getInitialProps as it's the newest recommended way to use Next: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props

// Before:

export default function Page (props) {
  return { ... }
}

Page.getInitialProps = (ctx: NextPageContext) => {
}


// Now:

export const getServerSideProps = withServerErrorHandler((ctx) => {
  return { props: { ... } }
})

const Page: NextPageWithInferredProps<typeof getServerSideProps> = (props) => {
  // Now all the props are typed
}

export default Page

The only caveat is that you have to ensure that getServerSideProps is exported on the apps that consume ui-components:

// Before:
import { default } from '@ui-components/pages/example'

// Now:
import { default, getServerSideProps } from '@ui-components/pages/example'

The only exceptions are _app.tsx and _error.tsx. These still use getInitialProps because getServerSideProps is not supported as of now:
https://nextjs.org/docs/advanced-features/custom-app#caveats
https://nextjs.org/docs/advanced-features/custom-error-page#caveats

It also ensures that all the props coming from getServerSideProps are typed.

Also fixes a few typing issues along the way as the files were being migrated, most notably the groups/rules.tsx page.

Checklists

Development

• Application changes have been tested appropriately

Impact

• Code follows company security practices and guidelines
• Security impact of change has been considered
• Performance impact of change has been considered
• Possible migration needs considered (model migrations, config migrations, etc.)

Please explain any security, performance, migration, or other impacts if relevant:

Code review

• Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached where applicable.
• Relevant tags have been added to the PR (bug, enhancement, internal, etc.)

[evantahler]

Awesome! This kind of consistency and better types will hopefully make it easier to avoid bugs, and for folks to know the "right way" to do things

[krishnaglick]

I think there could be some speed efficiencies by combining async requests in the getServerSideProps call, but that might be worth ticketing versus doing that work here.

[krishnaglick]

Nitpick, though I know this was around before:

Suggested change

	let exportProcessorId: string;
	let exportProcessorId = "";

[edmundito]

Will leave as is because it was like that before. I'm not sure if this will cause the action to fail later on, and I'd like not to specifically test this.

[krishnaglick]

👍

[krishnaglick]

Nitpick, and I'm not sure if this is more efficient.

Suggested change

	let scheduleRuns = [];
	let schedules = [];
	if (sources) {
	schedules = sources
	.filter(
	(source) => source.appId === app.id && source.schedule?.refreshEnabled
	)
	.map((source) => source.schedule);
	const scheduleRuns = [];
	const schedules = [];
	if (sources) {
	schedules.push(...sources
	.filter(
	(source) => source.appId === app.id && source.schedule?.refreshEnabled
	)
	.map((source) => source.schedule));

[krishnaglick]

Suggested change

	.sort((a, b) => (a.name > b.name ? 1 : -1));
	.sort((a, b) => (a.name > b.name ? 1 : a.name < b.name ? -1 : 0));

[krishnaglick]

I think it's probably worth putting what RunsList.hydrate does into another method and calling that method from both places, rather than doing it like it.

[edmundito]

@krishnaglick I appreciate all the suggestions but I wanted to focus this on migrating from one function to another without making a lot of changes unless it was necessary (like that groups rules file). I rather not apply these changes since every I apply I should re-test.

[krishnaglick]

@krishnaglick I appreciate all the suggestions but I wanted to focus this on migrating from one function to another without making a lot of changes unless it was necessary (like that groups rules file). I rather not apply these changes since every I apply I should re-test.

I feel like these two should be addressed:
Commented Code
Incorrect Sorting

Everything else can be ticketed or counted as a nitpick I'd say.

[edmundito]

Rebased and address @krishnaglick's comments.