-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
feat: create release schedule component #8529
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { create } from 'lts'; | ||
|
|
||
| import provideReleaseSchedule from '#site/next-data/providers/releaseSchedule'; | ||
|
|
||
| import type { FC } from 'react'; | ||
|
|
||
| const MONTH = 30 * 24 * 3_600_000; | ||
|
|
||
| const ReleaseSchedule: FC = async () => { | ||
| const schedule = await provideReleaseSchedule(); | ||
|
|
||
| // eslint-disable-next-line react-hooks/purity | ||
| const now = Date.now(); | ||
|
|
||
| const threeMonthsAgo = new Date(now - 3 * MONTH); | ||
| const sixMonthsFromNow = new Date(now + 6 * MONTH); | ||
|
|
||
| const svg = create({ | ||
| data: schedule, | ||
| queryStart: threeMonthsAgo, | ||
| queryEnd: sixMonthsFromNow, | ||
| animate: true, | ||
| excludeMain: false, | ||
| projectName: 'Node.js', | ||
| currentDateMarker: 'red', | ||
| }); | ||
|
|
||
| return ( | ||
| <div | ||
| dangerouslySetInnerHTML={{ __html: svg.html() }} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I understand the reason for this, is there a way to avoid this potentially dangerous situation?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not that I'm aware of,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably then recommend finding a way to make this return a react node instead of raw html. Who maintains 'lts'? Can we make a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's on Node.js domain
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can return variables like
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
build wg but not really maintained. IMO it's can be a good idea to make it live here under web-infra team
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably should have co-ownership, but this feels like smth that should be owned by releasers or build primarily.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@avivkeller @ovflowd I think @AugustinMauroy has a great point. The @nodejs/releasers showed they already have plans of getting rid of SVG, so maybe it's time for the @nodejs/nodejs-website to create our own version.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @avivkeller I don't think they have plans to get rid of the svg ;) what Richard stated was that the PR Guilherme originally opened didn't make sense for them on their PoV and should be done on our side.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should have competing repositories for the lts svg source of truth tbh. All I'm saying is we can have shared ownership of the lts package if needed. Or make a new package that extends that one at maximum, but I'd like to avoid that route...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to be honest, I'm fine with this as long as server-side-rendering works.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Me too. We can refactor this by creating a component that does not use d3-node, but we'll end up with more duplicated code.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine too |
||
| className="h-auto w-auto" | ||
| /> | ||
|
Comment on lines
+28
to
+32
|
||
| ); | ||
| }; | ||
|
|
||
| export default ReleaseSchedule; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { RELEASE_SCHEDULE_URL } from '#site/next.constants.mjs'; | ||
| import { fetchWithRetry } from '#site/util/fetch'; | ||
|
|
||
| async function fetchReleaseSchedule() { | ||
| const response = await fetchWithRetry(RELEASE_SCHEDULE_URL); | ||
|
|
||
| return response.json(); | ||
| } | ||
|
|
||
| export default fetchReleaseSchedule; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import { cache } from 'react'; | ||
|
|
||
| import generateReleaseSchdule from '#site/next-data/generators/releaseSchedule.mjs'; | ||
|
|
||
| export default cache(generateReleaseSchdule); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,7 @@ | |
| "feed": "~5.1.0", | ||
| "github-slugger": "~2.0.0", | ||
| "gray-matter": "~4.0.3", | ||
| "lts": "github:araujogui/lts-schedule#refactor-website", | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use the upstream package once this pr is merged
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding blocked until then |
||
| "mdast-util-to-string": "^4.0.0", | ||
| "next": "16.0.10", | ||
| "next-intl": "~4.5.3", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is showing a warning that I'm not sure how to solve: