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.

Sign up for GitHub

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

Jump to bottom

nested pages are not returned in order as they are on the filesystem #386

Closed
1 of 5 tasks
thescientist13 opened this issue Jul 11, 2020 · 5 comments · Fixed by #449
Closed
1 of 5 tasks

nested pages are not returned in order as they are on the filesystem #386

thescientist13 opened this issue Jul 11, 2020 · 5 comments · Fixed by #449
Assignees
@thescientist13
Labels
alpha.2 bug Something isn't working CLI P0 Critical issue that should get addressed ASAP v0.10.0
Milestone
1.0

Comments

@thescientist13
Copy link
Member

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

Getting intermittent test failure similar to the issue observed in #271 , in that the order of pages returned did not seem to returned from the graph in the same order as they are on the filesystem, in particular when there are multiple files in nested directories. I notice our test case does not act and trying to do a map in the template, e.g.

While it is always true that users can do their filtering and sorting client side, Greenwood should at least to a consistent, default, predictable return order.

Details

For example, when working on the PR for #371 I created a directory structure for the graph test case, similar to my website in that it had nested directories like below

src/
  index.md
  pages/
    blog/
      first-post.md
      second-post.md
    index.md

And in a template, listing the posts on the page

class BlogTemplate extends LitElement {
  ...

  async connectedCallback() {
    super.connectedCallback();
    const response = await client.query({
      query: ChildrenQuery,
      variables: {
        parent: 'blog'
      }
    });

    this.posts = response.data.children.filter(post => {
      return post.filePath.indexOf('/blog/index') < 0;
    });
  }

  /* eslint-disable indent */
  render() {
    const { posts } = this;

    return html`

      <div class='container'>
          <ul>
            ${posts.map((post) => {
              return html`
                <li>
                  <a href="${post.link}" title="Click to read my ${post.title} blog post">${post.title}</a>
                </li>
              `;
            })}
          </ul>
      </div>
    `;
  }
  /* eslint-enable */
}

customElements.define('page-template', BlogTemplate);

However, intermittently, i would get this error

551 passing (5m)
  4 failing

  1) Build Greenwood With:
       Data from GraphQL
         Home (Page Template) w/ Navigation Query
           should have a expected NavigationQuery output in the <header> tag:

      AssertionError: expected '/blog/second-post/' to equal '/blog/first-post/'
      + expected - actual

      -/blog/second-post/
      +/blog/first-post/

      at Context.<anonymous> (packages/cli/test/cases/build.data.graph/build.data.graph.spec.js:114:57)

  2) Build Greenwood With:
       Data from GraphQL
         Blog Page (Template) w/ Navigation and Children Query
           should have expected navigation links in the <header> tag tag when using NavigationQuery:

      AssertionError: expected '/blog/second-post/' to equal '/blog/first-post/'
      + expected - actual

      -/blog/second-post/
      +/blog/first-post/

      at Context.<anonymous> (packages/cli/test/cases/build.data.graph/build.data.graph.spec.js:156:57)

  3) Build Greenwood With:
       Data from GraphQL
         Blog Page (Template) w/ Navigation and Children Query
           should have expected blog posts links in the <body> tag when using ChildrenQuery:

      AssertionError: expected '/blog/second-post/' to equal '/blog/first-post/'
      + expected - actual

      -/blog/second-post/
      +/blog/first-post/

      at Context.<anonymous> (packages/cli/test/cases/build.data.graph/build.data.graph.spec.js:175:57)

  4) Build Greenwood With:
       Data from GraphQL
         Blog Post Pages (Post Template) w/ custom Graph Query
           should have expected navigation links in the <header> tag tag when using NavigationQuery:

      AssertionError: expected '/blog/second-post/' to equal '/blog/first-post/'
      + expected - actual

      -/blog/second-post/
      +/blog/first-post/

      at Context.<anonymous> (packages/cli/test/cases/build.data.graph/build.data.graph.spec.js:219:56)

Interestingly, there is a nested test case, but it doesn't have multiple files in the nested directory, like described above. I would start by updating that test case to something like

src/
  index.md
  pages/
    blog/
       2019/
        first-post.md
        second-post.md
      2020/
        first-post.md
        second-post.md
        third-post.md
    index.md

And return in the following order

  1. _/blog/2019/first-post.md/
  2. _/blog/2019/second-post.md/
  3. _/blog/2020/first-post.md/
  4. _/blog/2020/second-post.md/
  5. _/blog/2020/third-post.md/
  6. _/blog/index.md/
@thescientist13 thescientist13 added bug Something isn't working CLI labels Jul 11, 2020
@thescientist13 thescientist13 added this to the MVP milestone Jul 11, 2020
@thescientist13 thescientist13 self-assigned this Jul 11, 2020
@thescientist13 thescientist13 mentioned this issue Jul 11, 2020
Merged
6 tasks
@thescientist13
Copy link
Member Author

thescientist13 commented Jul 11, 2020 •
edited
Loading

So I ran the PR at commit 385eeeb five time in GitHub Actions and re-running the job (though it doesn't actually display that it ran five time... :/) and also ran it five times locally and was not able to replicate it. Maybe just had a local change that threw off the test that I fixed and didn't realize it? Removing bug label for now and adding question. Will review on the next meeting,

@thescientist13 thescientist13 added the question Further information is requested label Jul 11, 2020
@thescientist13 thescientist13 removed this from the MVP milestone Jul 11, 2020
@hutchgrant
Copy link
Member

I too was unable to replicate this bug on a fresh project with the same suggested file structure. Can't test a fix if can't replicate. Test it on a slower machine, with larger pages, run it more times until you break it? I don't know.

@thescientist13
Copy link
Member Author

Could have just made a change or forgot to save a file. Was at the tail end of a bunch of work so who knows. Glad to hear it's working for you too. 🍀

@thescientist13 thescientist13 removed the bug Something isn't working label Jul 12, 2020
@thescientist13
Copy link
Member Author

thescientist13 commented Jul 15, 2020 •
edited
Loading

Not one failing build in #383 even with a bunch more branch runs, so going to consider this cannot reproduce.

@thescientist13 thescientist13 mentioned this issue Dec 12, 2020
Closed
@thescientist13
Copy link
Member Author

Sad to say, but it looks like this issue may still be relevant :/
thegreenhouseio/www.thegreenhouse.io#174 (comment)

@thescientist13 thescientist13 added the bug Something isn't working label Dec 16, 2020
@thescientist13 thescientist13 added P0 Critical issue that should get addressed ASAP and removed question Further information is requested labels Dec 16, 2020
@thescientist13 thescientist13 mentioned this issue Dec 18, 2020
Merged
2 tasks
@thescientist13 thescientist13 linked a pull request Dec 25, 2020 that will close this issue
Bug/issue 386 graph not matching filesystem order #449
Merged
2 tasks
@thescientist13 thescientist13 mentioned this issue Dec 30, 2020
Merged
12 tasks
@thescientist13 thescientist13 added this to the MVP milestone Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thescientist13 thescientist13

Labels
alpha.2 bug Something isn't working CLI P0 Critical issue that should get addressed ASAP v0.10.0
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

Bug/issue 386 graph not matching filesystem order ProjectEvergreen/greenwood
2 participants
@thescientist13 @hutchgrant