imprv: Improve default behavior of skipSSR #7927
Conversation
apps/app/src/server/models/page.ts
Outdated
| @@ -959,6 +960,37 @@ schema.statics.findNonEmptyClosestAncestor = async function(path: string): Promi | |||
| return ancestors[0]; | |||
| }; | |||
|
|
|||
| export async function calculateAndUpdateLatestRevisionBodyLength(page: PageDocument): Promise<number | null> { | |||
| const latestRevisionData = await mongoose.model('Revision').findById(page.revision, { body: 1 }); | |||
There was a problem hiding this comment.
model 層で populate は NG だと思っていました。populate するように変更してみました。
apps/app/src/server/models/page.ts
Outdated
| // Infer the type as Omit<PageDocument, never> due to the population | ||
| // Cast the type back to PageDocument to restore the original type | ||
| // eslint-disable-next-line rulesdir/no-populate | ||
| const populatedPageDocument = await this.populate('revision', 'body') as PageDocument; |
There was a problem hiding this comment.
const populatedPageDocument = await this.populate<PageDocument>('revision', 'body');
apps/app/src/server/models/page.ts
Outdated
|
|
||
| if (typeof populatedPageDocument.revision === 'string') { | ||
| throw new Error('Failed to populate revision field in the page document.'); | ||
| } |
There was a problem hiding this comment.
@growi/corepackage のisPopulatedを使うassert(isPopulated(...))でOK
| /* | ||
| * get latest revision body length | ||
| */ | ||
| schema.methods.getLatestRevisionBodyLength = async function(this: PageDocument): Promise<number | null | undefined> { |
There was a problem hiding this comment.
-
this.latestRevisionBodyLengthがもともと number | undefined 型なので undefined も許容しないと最後のreturn this.latestRevisionBodyLength;で型エラーになるためこうしました。
確かに値が入っているか null かどうかなことは getLatestRevisionBodyLength すれば分かるので undefined が返りえるのが気持ち悪い感じもします。
ただ考えてみると、実際 latestRevisionBodyLength フィールドに null を格納するわけではないのでむしろ undefined を返すほうが正しいんでしょうかね？ -
calculateAndUpdateLatestRevisionBodyLength が結果を返すようにすれば解決しそうですが、calculateAndUpdate の接頭語のメソッドに結果を返す責務も持たせていいのかわからなかったのでこうしていました。
apps/app/src/pages/utils/commons.ts
Outdated
| const latestRevisionBodyLength = await page.getLatestRevisionBodyLength(); | ||
|
|
||
| if (latestRevisionBodyLength == null) { | ||
| return false; |
There was a problem hiding this comment.
ここ return true では？
過去の revision 閲覧時は多少 UX 落ちても良くて、それより重いかもしれないデータを SSR しない方が安全だよね。
There was a problem hiding this comment.
そうか、過去の revision を見る場合は安全ですね。
return true で SSR しないようにします。
|
reg-suit detected visual differences. Check this report, and review them. 🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴 🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵 What do the circles mean?The number of circles represent the number of changed images.🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items How can I change the check status?If reviewers approve this PR, the reg context status will be green automatically. |
実装方針