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

[Next Sitemap Config] Missing <loc>https://www.bytebase.com</loc> in the generated sitemap #192

Closed
gustaveWPM opened this issue Sep 28, 2023 · 4 comments

Comments

@gustaveWPM
Copy link
Contributor

gustaveWPM commented Sep 28, 2023

Hello.

I noticed that <loc>https://www.bytebase.com</loc> is missing in the generated sitemap.

CTRL+F https://www.bytebase.com<
https://www.bytebase.com/sitemap-0.xml


In this PR:
#190

And more specifically, in this commit:
28fa929

You should beware of your custom transformation.

transform: async (config, path) => {
  if (path.startsWith('/en')) {
      path = path.substring(3);
  }
}

If your homepage is served at https://www.bytebase.com/en, then rewritten https://www.bytebase.com, you're currently losing it in your generated Sitemap with this transformation. (Because '/en'.substring(3) is equal to ''.)

Here is a proposal of fix:

⚠️ WRONG APPROACH: See #192 (comment)

// next-sitemap.config.js

// * ...
const DEFAULT_LANGUAGE = 'en';

// * ...
transform: (config, path) => {
  const DEFAULT_LANGUAGE_NEEDLE = '/' + DEFAULT_LANGUAGE;
  if (path.startsWith(DEFAULT_LANGUAGE_NEEDLE)) {
    if (path.length === DEFAULT_LANGUAGE_NEEDLE.length) path = '/'; // * ... RIGHT there! :D
    else path = path.substring(DEFAULT_LANGUAGE_NEEDLE.length);
  }
  // * ...
}

Also, I think we can get rid off the async function expression since the await operator isn't used yet in the transform function.
⬆️ (Still valid suggestion, unlike the code snippet just above.)

Cheers!

@gustaveWPM
Copy link
Contributor Author

(Ongoing PR...)

@gustaveWPM gustaveWPM changed the title [Next Sitemap Config] <loc>Missing bytebase.com</loc> in the generated sitemap [Next Sitemap Config] Missing <loc>bytebase.com</loc> in the generated sitemap Sep 28, 2023
@gustaveWPM
Copy link
Contributor Author

gustaveWPM commented Sep 28, 2023

Hmmm, actually, I think I've messed up my fix proposal.

Here is a new one:

// next-sitemap.config.js

// * ...
const DEFAULT_LANGUAGE = 'en';

// * ...
transform: (config, path) => {
  const defaultLanguageNeedle = '/' + DEFAULT_LANGUAGE;
  const defaultLanguageEnvelopeNeedle = defaultLanguageNeedle + '/';
  const defaultLanguageNeedleLen = defaultLanguageNeedle.length;
  if (path.startsWith(defaultLanguageEnvelopeNeedle)) {
    path = path.substring(defaultLanguageNeedleLen);
  } else if (path === defaultLanguageNeedle) {
    path = '/';
  }
  // * ...
}

ℹ️ I think we can get rid off the async function expression since the await operator isn't used yet in the transform function.

I think also that it is safer to check whether the path starts with /en/ or is just /en, and adjust the transformation as accurately as possible (so that /en/* becomes /*, and just /en becomes /).

@gustaveWPM
Copy link
Contributor Author

Sorry, as I'm a complete beginner, I've made a lot of mistakes and I've had to edit my PR commit and my messages too many times.

It's OK now. 'night!

@gustaveWPM gustaveWPM changed the title [Next Sitemap Config] Missing <loc>bytebase.com</loc> in the generated sitemap [Next Sitemap Config] _Maybe_ missing <loc>bytebase.com</loc> in the generated sitemap Sep 28, 2023
@gustaveWPM gustaveWPM changed the title [Next Sitemap Config] _Maybe_ missing <loc>bytebase.com</loc> in the generated sitemap [Next Sitemap Config] Maybe missing <loc>bytebase.com</loc> in the generated sitemap Sep 28, 2023
@gustaveWPM gustaveWPM changed the title [Next Sitemap Config] Maybe missing <loc>bytebase.com</loc> in the generated sitemap [Next Sitemap Config] Maybe missing <loc>https://bytebase.com</loc> in the generated sitemap Sep 28, 2023
@gustaveWPM gustaveWPM changed the title [Next Sitemap Config] Maybe missing <loc>https://bytebase.com</loc> in the generated sitemap [Next Sitemap Config] Missing <loc>https://www.bytebase.com</loc> in the generated sitemap Sep 28, 2023
@gustaveWPM
Copy link
Contributor Author

#193 (review)

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

1 participant