feat: closes #69 and & adds sitemap: false option to pages#70
feat: closes #69 and & adds sitemap: false option to pages#70SnirShechter wants to merge 2 commits intonuxt-community:devfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #70 +/- ##
===================================
Coverage 100% 100%
===================================
Files 1 1
Lines 3 3
===================================
Hits 3 3Continue to review full report at Codecov.
|
|
@NicoPennec |
Codecov Report
@@ Coverage Diff @@
## dev #70 +/- ##
===================================
Coverage 100% 100%
===================================
Files 1 1
Lines 3 3
===================================
Hits 3 3Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## dev #70 +/- ##
===================================
Coverage 100% 100%
===================================
Files 1 1
Lines 3 3
===================================
Hits 3 3Continue to review full report at Codecov.
|
|
this looks great! |
|
@pi0 @NicoPennec |
|
Thanks @SnirShechter for your proposal 🙏 Sorry for the delay, I will test and review your PR this week! |
NicoPennec
left a comment
There was a problem hiding this comment.
Can you make a benchmark before / after the addition of your feature?
I want to validate the right performance with the "acorn" parser during the generation of sitemap.
| } | ||
| if (r.path !== '') { | ||
| routes.push(path + r.path) | ||
| routes.push({ ...r, url: path + r.path }) |
There was a problem hiding this comment.
This update can create a breaking change in the usage of the filter option of the sitemap module, no ?
if yes, your commit message must respect the conventional commit to bump the release version as "major" update by the release script
| @@ -0,0 +1,4 @@ | |||
| module.exports = { | |||
| COMPONENT_OPTIONS_BLOCK: 'script', | |||
There was a problem hiding this comment.
In any <page>.vue file, it will always be a script element.
Why create a constant?
| @@ -0,0 +1,4 @@ | |||
| module.exports = { | |||
| COMPONENT_OPTIONS_BLOCK: 'script', | |||
| COMPONENT_OPTIONS_KEY: 'sitemap' | |||
There was a problem hiding this comment.
To be customisable, you can set that key in the sitemap configuration in nuxt.config.js instead of create a constant.
| const compiler = require('vue-template-compiler') | ||
| // const { COMPONENT_OPTIONS_BLOCK, COMPONENT_OPTIONS_KEY } = require('constants') | ||
|
|
||
| function extractComponentOptions (path, blockName, key) { |
There was a problem hiding this comment.
I think you should rename the function and file from extractComponentOptions to extractPageOptions, because it is relative to the *.vue file in the Pages folder.
| </template> | ||
| <script> | ||
| export default { | ||
| sitemap:false |
There was a problem hiding this comment.
Why limit the sitemap option to a simple boolean for exclude / include a page from the sitemap?
Maybe you can support the following options in addition of sitemap: true :
<script>
export default {
sitemap: {
changefreq: 'daily',
priority: 1,
lastmod: new Date(),
// ...
}|
@NicoPennec |
|
just similar implementation with no extra deps #101 |
Closes #69 - adds data to routes in the filter function.
Also closes #c55 (for some reason the issue was not posted in github).