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.

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

#365 created page with xml(rss) #432

Closed
wants to merge 2 commits into from

Conversation

Aleksandr-Bondarev
Copy link
Contributor

@Aleksandr-Bondarev Aleksandr-Bondarev commented Sep 4, 2022

Добавил в next-app/pages feed.xml.js
Задача #424

<!-- <turbo:adNetwork></turbo:adNetwork> -->
<item turbo="true">
<turbo:extendedHtml>true</turbo:extendedHtml>
<link>http://localhost:3000/makefile-as-task-runner/</link>
Copy link
Contributor

Choose a reason for hiding this comment

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

@Aleksandr-Bondarev кажется так не пойдет. Предлагаю глянуть сюда

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Забыл отписаться, сделал генерацию по примеру из статьи, валидный rss отдаётся по роуту /feed.xml, проверял с помощью RSS Feed Reader

});
});

fs.writeFileSync("./public/feed.xml", feed.rss2());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fs.writeFileSync("./public/feed.xml", feed.rss2());
return fsp.writeFile("./public/feed.xml", feed.rss2());

author: {
name: post.author,
},
// image: post.image,
Copy link
Contributor

Choose a reason for hiding this comment

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

Не используемый код лучше удалить

Copy link
Contributor

Choose a reason for hiding this comment

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

Еще кажется, что дата публикации все же должна быть:

<pubDate>Fri, 10 Jun 2022 00:00:00 +0000</pubDate>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Пост не содержит информацию о дате публикации. В каждом файле есть ключи title, subtitle, description, image и author. Или дату можно получить как-то ещё?

const date = new Date();

const feed = new Feed({
title: "Hexlet Guides",
Copy link
Contributor

Choose a reason for hiding this comment

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

Такие параметры лучше брать из конфига

const posts = await getPublishedPosts(locale);
const postsToShow = posts.filter(({ hidden = false }) => !hidden);

const siteURL = 'http://localhost:3000';
Copy link
Contributor

Choose a reason for hiding this comment

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

Кажется так быть не должно - https://github.com/Hexlet/hexletguides.github.io/blob/gh-pages/feed.xml#L8:

<link>https://guides.hexlet.io/</link>

@@ -18,6 +18,7 @@ const Home = ({ posts }) => {
export const getStaticProps = async ({ locale }) => ({
props: {
posts: await getPostsList(locale),
...await generateRssFeed(locale),
Copy link
Contributor

@seth2810 seth2810 Sep 12, 2022

Choose a reason for hiding this comment

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

Есть ощущение, что генерация должна выполняться не здесь, а в отдельной команде перед деплоем (см. как сделан sitemap)

@see

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seth2810 Привет, я попытался сделать пребилд скрипт (“prebuild”: “node <путь к скрипту>”), но так как в package.json не указан “type”: ”module”, команда падает с ошибкой. Так как в скрипте используется код, который выполняется в рамках рантайма next.js, все функции (с зависимостями) переписать на CommonJS модули тоже не вышло. Попытки использовать расширение .mjs и флаги типа —experimental-modules так же не привели к успеху. Пожалуйста, подскажи, как можно решить эту проблему

Copy link
Contributor

Choose a reason for hiding this comment

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

@Aleksandr-Bondarev предлагаю тебе запушить свой код, пусть и не рабочий, чтобы я мог смотреть предметно 😉

Copy link
Contributor

@seth2810 seth2810 Sep 18, 2022

Choose a reason for hiding this comment

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

@Aleksandr-Bondarev учитывая, что будет необходимо генерировать фиды под разные локали, нашел другой способ позволяющий это сделать более привычным для next.js путем. Еще там выяснилось, что обычный feed нам не подходит, и лучше использовать turbo-rss. Завтра постараюсь прислать PR в твой репозиторий с правками. Так же переделаем заодно генерацию sitemap's, чтобы было единообразно 🤝

@seth2810 seth2810 mentioned this pull request Sep 15, 2022
@seth2810
Copy link
Contributor

@Aleksandr-Bondarev поправишь?

@Aleksandr-Bondarev
Copy link
Contributor Author

@seth2810 Конечно. Постараюсь успеть за выходные

@seth2810
Copy link
Contributor

Перенес изменения вместе со своими правками в #448

@Aleksandr-Bondarev можешь закрыть PR плз.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants