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

Feature: Single & Multiple sitemaps #26

Conversation

Projects
None yet
8 participants
@WilliamDASILVA
Copy link

commented Apr 27, 2018

Able to create a single sitemap like before by assignign a single object as the module option or pass an array of objects, each object being a single sitemap.

Switched from jest to ava for unit tests.

Fixes #6 and fixes #19

@WilliamDASILVA

This comment has been minimized.

Copy link
Author

commented May 2, 2018

@NicoPennec Saw you added more tests in master, should I remove the migration to Ava?

@NicoPennec

This comment has been minimized.

Copy link
Member

commented May 3, 2018

Thank you for this pull request. It will be an useful feature :)

But there are some points of divergence.
I wish to keep Jest instead of AVA, to follow the Nuxt ecosystem test suit. (1)
Besides, the minimum version of engine from package.json must be compliant with the current Nuxt.js requirement, so >=8.0.0 (2)
Then, Babel is not usefull with node 8.0.0, because it supports natively ES2016, so I try to keep a pure vanilla JS code base.

I'm working on a refactoring based on the new hooks from Nuxt >=1.x, that will be release very soon. It's the reason of my commit with new tests.

It’s planned to support multiple sitemaps. I should pick up your method for this update. Nice job

(1)
module template
https://github.com/nuxt-community/module-template
jest
https://github.com/nuxt/nuxt.js/blob/dev/package.json#L152
https://stackshare.io/stackups/jest-vs-ava (popularity)

(2)
node engine
https://github.com/nuxt/nuxt.js/blob/dev/package.json#L67

@NicoPennec NicoPennec self-requested a review Dec 8, 2018

@Livog

This comment has been minimized.

Copy link

commented Dec 19, 2018

Would be awesome to see this feature as if you are doing a big project like myself. I can't have sitemaps that are bigger than 50MB or 50000 tags.

@WilliamDASILVA WilliamDASILVA force-pushed the WilliamDASILVA:feature/multiple-sitemaps branch from b25b802 to beae5f9 Dec 19, 2018

@WilliamDASILVA

This comment has been minimized.

Copy link
Author

commented Dec 19, 2018

Hello,

I've updated the PR to remove my extra-changes (Ava, babel, Node version, ...) and updated it with the latest master changes.

If you (@NicoPennec) could take a look, that would be cool.

Thanks @Livog for reminding me that this PR still existed.

@manniL

This comment has been minimized.

Copy link
Member

commented Jan 12, 2019

Show resolved Hide resolved src/index.js
Show resolved Hide resolved src/index.js Outdated
@amjadkhan896

This comment has been minimized.

Copy link

commented Feb 12, 2019

@manniL @NicoPennec This feature is completed or still pending?. Because I also need this feature.
Thanks

Update the static path destination
Co-Authored-By: WilliamDASILVA <william.da.silva@outlook.com>
@codecov-io

This comment has been minimized.

Copy link

commented Feb 14, 2019

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #26   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      2    +1     
  Lines           2      4    +2     
=====================================
+ Hits            2      4    +2
Impacted Files Coverage Δ
test/fixture/multiple-sitemaps.nuxt.config.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80b8bf3...993f189. Read the comment docs.

Show resolved Hide resolved test/fixture/static/second-sitemap.xml Outdated
Show resolved Hide resolved src/index.js Outdated

@manniL manniL self-requested a review Feb 22, 2019

@manniL

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

cc @NicoPennec ☺️

@manniL

manniL approved these changes Feb 22, 2019

@manniL manniL closed this Mar 7, 2019

@manniL manniL reopened this Mar 7, 2019

@manniL

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

FYI: Had to close and reopen the PR to trigger CircleCI

@samturrell

This comment has been minimized.

Copy link

commented Apr 8, 2019

What's this feature waiting on, out of interest?

@manniL

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

@WilliamDASILVA could you resolve the merge conflits? ☺️

@maxkobzev

This comment has been minimized.

Copy link

commented Apr 11, 2019

Would be great to add option to create sitemapIndex for multiple sitemaps

@amjadkhan896

This comment has been minimized.

Copy link

commented Apr 30, 2019

Hi all,
I appreciate your hard work for making such a nice plugin for us. I would ask is this feature completed or still under development. Because as @manniL mentioned up i used my code like below.


 sitemap: [
        {
            path: '/sitemap.xml',
            cacheTime: 1000 * 60 * 15,
            hostname: "https://xyz.com",
            routes() {
                //console.log(process.env.apiUrl);
                return axios.get('https://api.xyz.com/api/get-sitemap', {
                    httpsAgent: new https.Agent({
                        rejectUnauthorized: false
                    })
                })
                    .then(res => res.data.map(cat => {
                        const path = 'https://xyz.com/' + cat.url
                        // console.log(cat);
                        return {
                            url: path,
                            changefreq: cat.frequency,
                            priority: cat.priority,
                            lastmodISO: cat.lastModData
                        };

                    }));

            }
        },
        {
            path: '/second-sitemap.xml',
            cacheTime: 1000 * 60 * 15,
            hostname: "https://xyz.com",
            routes() {
                //console.log(process.env.apiUrl);
                return axios.get('https://api.xyz.com/api/get-sitemap', {
                    httpsAgent: new https.Agent({
                        rejectUnauthorized: false
                    })
                })
                    .then(res => res.data.map(cat => {
                        const path = 'https://xyz.com/' + cat.url
                        // console.log(cat);
                        return {
                            url: path,
                            changefreq: cat.frequency,
                            priority: cat.priority,
                            lastmodISO: cat.lastModData
                        };

                    }));

            }
        },

    ],

But the first sitemap generated was with empty URLs. and the second responds with not found Error.
We are eagerly waiting for this feature.
Thanks Once again.

@NicoPennec

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

As explain before the current PR does not match my goal of the multi-sitemap. Thanks again WilliamDASILVA for your proposal.

Then, as explain on another issue, a refactoring with multi-sitemap and sitemap-index is WIP and will be ready in short term.

@NicoPennec NicoPennec closed this Apr 30, 2019

@amjadkhan896

This comment has been minimized.

Copy link

commented Apr 30, 2019

Thanks, @NicoPennec for the reply. Can you please specify which one is the exact PR. So that we go there and informed by the email once completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.