Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[meta-tags] 앱라우터용 메타태그 및 QaPageScript, DiscussionForumPostingScript를 추가합니다. #3500

Merged
merged 8 commits into from
Dec 20, 2024
Merged

[meta-tags] 앱라우터용 메타태그 및 QaPageScript, DiscussionForumPostingScript를 추가합니다. #3500

merged 8 commits into from
Dec 20, 2024

Conversation

dongoc
Copy link
Contributor

@dongoc dongoc commented Dec 12, 2024 •
edited
Loading

PR 설명

nextjs 앱라우터용 메타태그 및 QaPageScript, DiscussionForumPostingScript를 추가합니다.

변경 내역

  • pages-router, app-router, structured-data로 폴더를 구분했습니다.
  • structured-data에 QaPageScript, DiscussionForumPostingScript를 추가했습니다.
  • 기존 structured-data에서 head 태그를 제거하고 next/script 태그를 사용하도록 변경했습니다. 또한 파싱 에러를 해결하도록 수정했습니다.
  • structured-data를 가공하는 addSchemaType 함수를 수정합니다. (조건문이 잘못 설정되어 재귀가 돌고 있지 않았습니다.)

@dongoc dongoc self-assigned this Dec 12, 2024
@codecov Codecov
Copy link

codecov bot commented Dec 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 58 lines in your changes missing coverage. Please review.

Project coverage is 14.31%. Comparing base (f663988) to head (e09119e).
Report is 28 commits behind head on main.

Files with missing lines Patch % Lines
...rc/app-router/generate-facebook-open-graph-meta.ts 0.00% 12 Missing ⚠️
.../src/app-router/generate-facebook-app-link-meta.ts 0.00% 9 Missing ⚠️
packages/meta-tags/src/utils.ts 0.00% 6 Missing and 2 partials ⚠️
...src/app-router/generate-apple-smart-banner-meta.ts 0.00% 6 Missing ⚠️
.../src/app-router/generate-essential-content-meta.ts 0.00% 5 Missing ⚠️
...s/meta-tags/src/app-router/generate-common-meta.ts 0.00% 2 Missing ⚠️
...ags/src/app-router/generate-triple-default-meta.ts 0.00% 2 Missing ⚠️
...s/meta-tags/src/structured-data/article-script.tsx 0.00% 2 Missing ⚠️
...ags/src/structured-data/breadcrumb-list-script.tsx 0.00% 2 Missing ⚠️
...tructured-data/discussion-forum-posting-script.tsx 0.00% 2 Missing ⚠️
... and 4 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3500      +/-   ##
==========================================
- Coverage   14.37%   14.31%   -0.06%     
==========================================
  Files         729      737       +8     
  Lines       10323    10364      +41     
  Branches     3481     3505      +24     
==========================================
  Hits         1484     1484              
- Misses       8412     8454      +42     
+ Partials      427      426       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dongoc
Copy link
Contributor Author

dongoc commented Dec 16, 2024

release-canary

@github-actions GitHub Actions
Copy link

v14.0.7-pr-3500.7 has been published!

@dongoc
Copy link
Contributor Author

dongoc commented Dec 16, 2024

release-canary

@github-actions GitHub Actions
Copy link

v14.0.7-pr-3500.9 has been published!

@dongoc dongoc added this to the 14.1.0 milestone Dec 17, 2024
@dongoc
Copy link
Contributor Author

dongoc commented Dec 17, 2024

release-canary

@github-actions GitHub Actions
Copy link

v14.0.7-pr-3500.10 has been published!

@dongoc dongoc changed the title 앱라우터용 메타태그 및 QaPageScript, DiscussionForumPostingScript를 추가합니다. [meta-tags] 앱라우터용 메타태그 및 QaPageScript, DiscussionForumPostingScript를 추가합니다. Dec 17, 2024
@dongoc dongoc marked this pull request as ready for review December 17, 2024 08:57
@dongoc dongoc requested a review from a team as a code owner December 17, 2024 08:57
@dongoc dongoc requested review from kooinsung, guswl98 and dia-triple and removed request for a team December 17, 2024 08:57
dongoc
dongoc commented Dec 17, 2024
View reviewed changes
packages/meta-tags/src/structured-data/article-script.tsx
const articleScript = createScript(props, 'Article')

return (
<Script
Copy link
Contributor Author

@dongoc dongoc Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 기존 코드에서 head 태그 제거(nextjs 앱라우터는 head 태그를 지원하지 않습니다)
  • 일반 script 태그를 next/script 태그로 변경
  • 간혹 파싱 오류가 있어 dangerouslySetInnerHtml로 주입합니다. (관련 스레드)

dongoc
dongoc commented Dec 17, 2024
View reviewed changes
packages/meta-tags/src/utils.ts
@@ -58,22 +61,23 @@ function addSchemaType<T extends object>(originObj: T): T {
return mergeObj({ '@type': value }, obj)
}

if (key in SCHEMA_TYPE_MAP) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 조건문에 걸려 의도한대로 재귀가 돌고 있지 않았습니다. 해당 조건문을 key값을 확인하는 곳으로 옮겼습니다.

guswl98
guswl98 reviewed Dec 18, 2024
View reviewed changes
packages/meta-tags/src/app-router/generate-facebook-open-graph-meta.ts Outdated
Comment on lines 22 to 26
titleFromProps || process.env.NEXT_PUBLIC_DEFAULT_PAGE_TITLE || undefined
const description =
descriptionFromProps ||
process.env.NEXT_PUBLIC_DEFAULT_PAGE_DESCRIPTION ||
undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 title과 description 모두 || undefined는 없어도 되지 않나요?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e09119e 여기서 제거했습니다!

@dongoc
Copy link
Contributor Author

dongoc commented Dec 20, 2024

🙇‍♂️

@dongoc dongoc merged commit 3783e61 into main Dec 20, 2024
11 checks passed
@dongoc dongoc deleted the add/meta-tag branch December 20, 2024 01:43
@dongoc dongoc mentioned this pull request Dec 23, 2024
Merged
@dongoc dongoc modified the milestones: 14.1.0, 14.0.7 Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guswl98 guswl98 guswl98 approved these changes

@kooinsung kooinsung kooinsung approved these changes

@dia-triple dia-triple Awaiting requested review from dia-triple dia-triple is a code owner automatically assigned from titicacadev/frontend

Assignees

@dongoc dongoc

Labels
meta-tags
Projects
None yet
Milestone
14.0.7
Development

Successfully merging this pull request may close these issues.
3 participants
@dongoc @kooinsung @guswl98