Add language to the version part · facebook/docusaurus@ff11797

richardzcode · 2017-12-20T02:01:55Z

I thought id is already taking langPart. Let me do some tests

const id = langPart + versionPart + link.doc;

JoelMarcey · 2017-12-20T02:05:21Z

Without this, you get:

Error: It looks like you've enabled language support, but haven't provided translated files. The document with id: '-version-1.0.0-doc1' doesn't exist.
    at HeaderNav.makeLinks (/Users/joelm/dev/Docusaurus-primary/lib/core/nav/HeaderNav.js:137:17)
    at Array.map (<anonymous>)
    at HeaderNav.renderResponsiveNav (/Users/joelm/dev/Docusaurus-primary/lib/core/nav/HeaderNav.js:263:26)
    at HeaderNav.render (/Users/joelm/dev/Docusaurus-primary/lib/core/nav/HeaderNav.js:205:19)
    at /Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:793:21
    at measureLifeCyclePerf (/Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:73:12)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (/Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:792:25)
    at ReactCompositeComponentWrapper._renderValidatedComponent (/Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:819:32)
    at ReactCompositeComponentWrapper.performInitialMount (/Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:359:30)
    at ReactCompositeComponentWrapper.mountComponent (/Users/joelm/dev/Docusaurus-primary/node_modules/react-dom/lib/ReactCompositeComponent.js:255:21)

after enabling versioning as described here: https://docusaurus.io/docs/en/versioning.html

JoelMarcey · 2017-12-20T02:07:11Z

Although, what you say makes sense :/

richardzcode · 2017-12-20T02:07:12Z

You are right. I got that error now

JoelMarcey · 2017-12-20T02:12:23Z

On the surface this looks like we can't have langPart and versionPart being set at the same time in most scenarios.

richardzcode · 2017-12-20T02:22:43Z

Yes. I've just tested. The Metadata we got are two sets: ['doc1', 'doc2', ...] and ['en-version-1.0.0-doc1', 'en-version-1.0.0-doc2', ...]

So when translation disabled, langPart has to respect empty string, but versionPart has to have 'en' prefix.

A proper fix maybe in readMetadata to solve the versioned ids, so it would be ['version-1.0.0-doc1', 'version-1.0.0-doc2', ...].

This could be addressed in another PR, to have thorough review and test again

richardzcode · 2017-12-20T02:27:14Z

To be more clear. when translation disabled but versioning enabled. we are generating 'version 1.0.0' pages and 'version next' pages. For 'version next' it picks up metadata 'doc1' etc., for 'version 1.0.0' it picks up metadata 'en-version-1.0.0-doc1' etc.

Hope this explains better :p

JoelMarcey · 2017-12-20T03:09:33Z

I was just thinking, I think it is the first - that was causing the problem. Can't we just say something like:

const langPart = env.translation.enabled
        ? (this.props.language || 'en') + '-'
        : '';
      const versionPart =
        env.versioning.enabled && this.props.version !== 'next'
          ? (langPart !== '' ? '-' : '') +
            'version-' +
            (this.props.version || env.versioning.latestVersion) +
            '-'
          : '';
      const id = langPart + versionPart + link.doc;

If we don't have a langPart , we don't need the - in versionPart

Just a thought.

richardzcode · 2017-12-20T03:32:41Z

Yes that is one of the issue. Another issue is there is no 'version-1.0.0-doc1' alike in Metadata, instead have entries like 'en-version-1.0.0-doc1'. What we have right now works, but may not be absolutely correct depend on how we think about it. Translation is not enabled, but we have 'en-' as part of metadata id.

I think if we want to change this, it has to be two parts. fix Metadata, and then fix how we generate id.

JoelMarcey · 2017-12-20T04:11:01Z

Hi @richardzcode. Right. But wouldn't my fix above solve the case where translation is not enabled? You could even fix it like this using env.translation.enabled as the check with versionPart.

const langPart = env.translation.enabled
        ? (this.props.language || 'en') + '-'
        : '';
const versionPart =
        env.versioning.enabled && this.props.version !== 'next'
          ? (env.translation.enabled ? '-' : '') +
            'version-' +
            (this.props.version || env.versioning.latestVersion) +
            '-'
          : '';
const id = langPart + versionPart + link.doc;

richardzcode · 2017-12-20T04:42:56Z

if

translation.enabled === false &&
env.versioning.enabled === true &&
this.props.version === '1.0.0' &&
link.doc === 'doc1'

then

langPart = '';
versionPart = '' + 'version-' + '1.0.0' + '-' = 'version-1.0.0-'
id = '' + 'version-1.0.0-' + 'doc1' = 'version-1.0.0-doc1'

Totally agree, this is desired. Just as of now. in Metadata, 'version-1.0.0-doc1' does not exist, so Metadata[id] === false, so it would throw error.

Right now Metadata['en-version-1.0.0-doc1'] exists. So your earlier fix worked. I prefer your current fix but we have to fix Metadata too.

richardzcode · 2017-12-20T04:44:28Z

This is the Metadata I have in my test:

{ doc1:
   { id: 'doc1',
     title: 'Latin-ish',
     sidebar_label: 'Example Page',
     source: 'doc1.md',
     version: 'next',
     permalink: 'docs/next/doc1.html',
     localized_id: 'doc1',
     language: 'en',
     sidebar: 'docs',
     category: 'Docusaurus',
     next_id: 'doc2',
     next: 'doc2',
     next_title: 'document number 2' },
  doc2:
   { id: 'doc2',
     title: 'document number 2',
     source: 'doc2.md',
     version: 'next',
     permalink: 'docs/next/doc2.html',
     localized_id: 'doc2',
     language: 'en',
     sidebar: 'docs',
     category: 'First Category',
     next_id: 'doc3',
     next: 'doc3',
     previous_id: 'doc1',
     previous: 'doc1',
     previous_title: 'Latin-ish',
     next_title: 'This is document number 3' },
  doc3:
   { id: 'doc3',
     title: 'This is document number 3',
     source: 'doc3.md',
     version: 'next',
     permalink: 'docs/next/doc3.html',
     localized_id: 'doc3',
     language: 'en',
     sidebar: 'docs',
     category: 'Second Category',
     previous_id: 'doc2',
     previous: 'doc2',
     previous_title: 'document number 2' },
  doc4:
   { id: 'doc4',
     title: 'Other Document',
     source: 'exampledoc4.md',
     version: 'next',
     permalink: 'docs/next/doc4.html',
     localized_id: 'doc4',
     language: 'en',
     sidebar: 'docs-other',
     category: 'First Category',
     next_id: 'doc5',
     next: 'doc5',
     next_title: 'Fifth Document' },
  doc5:
   { id: 'doc5',
     title: 'Fifth Document',
     source: 'exampledoc5.md',
     version: 'next',
     permalink: 'docs/next/doc5.html',
     localized_id: 'doc5',
     language: 'en',
     sidebar: 'docs-other',
     category: 'First Category',
     previous_id: 'doc4',
     previous: 'doc4',
     previous_title: 'Other Document' },
  'en-version-1.0.0-doc1':
   { id: 'en-version-1.0.0-doc1',
     title: 'Latin-ish',
     sidebar_label: 'Example Page',
     original_id: 'doc1',
     source: 'version-1.0.0/doc1.md',
     permalink: 'docs/doc1.html',
     localized_id: 'version-1.0.0-doc1',
     language: 'en',
     version: '1.0.0',
     sidebar: 'version-1.0.0-docs',
     category: 'Docusaurus',
     next_id: 'doc2',
     next: 'en-version-1.0.0-doc2',
     next_title: 'document number 2' },
  'en-version-1.0.0-doc2':
   { id: 'en-version-1.0.0-doc2',
     title: 'document number 2',
     original_id: 'doc2',
     source: 'version-1.0.0/doc2.md',
     permalink: 'docs/doc2.html',
     localized_id: 'version-1.0.0-doc2',
     language: 'en',
     version: '1.0.0',
     sidebar: 'version-1.0.0-docs',
     category: 'First Category',
     next_id: 'doc3',
     next: 'en-version-1.0.0-doc3',
     previous_id: 'doc1',
     previous: 'en-version-1.0.0-doc1',
     previous_title: 'Latin-ish',
     next_title: 'This is document number 3' },
  'en-version-1.0.0-doc3':
   { id: 'en-version-1.0.0-doc3',
     title: 'This is document number 3',
     original_id: 'doc3',
     source: 'version-1.0.0/doc3.md',
     permalink: 'docs/doc3.html',
     localized_id: 'version-1.0.0-doc3',
     language: 'en',
     version: '1.0.0',
     sidebar: 'version-1.0.0-docs',
     category: 'Second Category',
     previous_id: 'doc2',
     previous: 'en-version-1.0.0-doc2',
     previous_title: 'document number 2' },
  'en-version-1.0.0-doc4':
   { id: 'en-version-1.0.0-doc4',
     title: 'Other Document',
     original_id: 'doc4',
     source: 'version-1.0.0/exampledoc4.md',
     permalink: 'docs/doc4.html',
     localized_id: 'version-1.0.0-doc4',
     language: 'en',
     version: '1.0.0',
     sidebar: 'version-1.0.0-docs-other',
     category: 'First Category',
     next_id: 'doc5',
     next: 'en-version-1.0.0-doc5',
     next_title: 'Fifth Document' },
  'en-version-1.0.0-doc5':
   { id: 'en-version-1.0.0-doc5',
     title: 'Fifth Document',
     original_id: 'doc5',
     source: 'version-1.0.0/exampledoc5.md',
     permalink: 'docs/doc5.html',
     localized_id: 'version-1.0.0-doc5',
     language: 'en',
     version: '1.0.0',
     sidebar: 'version-1.0.0-docs-other',
     category: 'First Category',
     previous_id: 'doc4',
     previous: 'en-version-1.0.0-doc4',
     previous_title: 'Other Document' } }

JoelMarcey · 2017-12-20T16:23:44Z

👍 Got it. When creating a version, it is prefixing the id with en by default. We don't want that.

I am working on a fix now.

JoelMarcey · 2017-12-20T18:38:15Z

I have a PR coming up.

-Original file line number
+Diff line change
@@ Expand Up / @@ -126,7 +126,8 @@ class HeaderNav extends React.Component { @@
             : '';
           const versionPart =
             env.versioning.enabled && this.props.version !== 'next'
-              ? '-version-' +
+              ? (this.props.language || 'en') +
+                '-version-' +
                 (this.props.version || env.versioning.latestVersion) +
                 '-'
               : '';
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commit

There are no files selected for viewing

14 comments on commit `ff11797`

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017 •

edited

Loading

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

Commit

There are no files selected for viewing

14 comments on commit ff11797

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

richardzcode commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

JoelMarcey commented on ff11797 Dec 20, 2017

Choose a reason for hiding this comment

14 comments on commit `ff11797`

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017 •

edited

Loading

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

richardzcode commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017

JoelMarcey commented on `ff11797` Dec 20, 2017