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

Support i18n module #7

Closed
5 tasks done
provok-me opened this issue Jan 25, 2023 · 5 comments
Closed
5 tasks done

Support i18n module #7

provok-me opened this issue Jan 25, 2023 · 5 comments

Comments

@provok-me
Copy link

provok-me commented Jan 25, 2023

Describe the bug

By default nuxt-simple-sitemap generates pages components urls.
This clashes with pre-rendered routes and i18n.

We should be able to disable default urls generation (prevent generateUrls function from running) without excluding them one by one.

Reproduction

Reproduction link

  • yarn build
  • open ./.output/public/sitemap.xml

System Info

System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 1.37 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 19.2.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.19.3 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 109.0.5414.87
    Firefox: 103.0.2
    Safari: 16.2

Used Package Manager

yarn

Validations

  • Follow our Code of Conduct
  • Read the Contributing Guide.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Check that this is a concrete bug. For Q&A, please open a GitHub Discussion instead.
  • The provided reproduction is a minimal reproducible of the bug.
@harlan-zw
Copy link
Collaborator

harlan-zw commented Jan 25, 2023

Hey, thanks for the issue!

I've added the inferStaticPagesAsRoutes to escape from using generateUrls, available in 1.0.5.

export default defineNuxtConfig({
  sitemap: {
    inferStaticPagesAsRoutes: false,
  }
})

I'll be looking to support i18n properly soon, if you have any other ideas on what the integration should look like, please let me know :)

@harlan-zw harlan-zw changed the title i18n problem. Pages components urls are included by default. Support i18n module Jan 25, 2023
@provok-me
Copy link
Author

@harlan-zw
It works like a charm, thanks 👌

PS In the README.md, it is said that the include property default value is undefined.
While in reality, its default value (src/module.ts line 69) is include: ['/**'].
You should update the README properties values to reflect reality.

@harlan-zw
Copy link
Collaborator

@harlan-zw It works like a charm, thanks ok_hand

PS In the README.md, it is said that the include property default value is undefined. While in reality, its default value (src/module.ts line 69) is include: ['/**']. You should update the README properties values to reflect reality.

Thanks, have updated the doc. Did you run into an issue with it being ['/**'*']?

@provok-me
Copy link
Author

Nope, no issues.

I find it weird to have include and exclude rules applied to both urls and prerendedRoutes since the challenges for those two arrays are different.

Plus, you run urlFilter two times.
The first time inside generateUrls, thus on the soon-to-be populated urls array.
The second time directly on the merged array [...urls, ...prerendedRoutes].
Consequently, you run the filter function twice on a section of the final URLs.

@harlan-zw
Copy link
Collaborator

harlan-zw commented Jan 27, 2023

Nope, no issues.

I find it weird to have include and exclude rules applied to both urls and prerendedRoutes since the challenges for those two arrays are different.

Plus, you run urlFilter two times. The first time inside generateUrls, thus on the soon-to-be populated urls array. The second time directly on the merged array [...urls, ...prerendedRoutes]. Consequently, you run the filter function twice on a section of the final URLs.

Agreed, the code isn't as good as it could be, optimised for getting it finished. Will refactor it at some point when I add a new feature. Always happy to receive PRs too :P

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

No branches or pull requests

2 participants