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

[gatsby-theme-notes] showing improper breadcrumbs when basePath is set to default value. #15493

Closed
jakewies opened this issue Jul 7, 2019 · 8 comments · Fixed by #16556
Closed

Comments

@jakewies
Copy link
Contributor

jakewies commented Jul 7, 2019

Description

I’ve been playing around with gatsby-theme-notes and noticed a small issue related to breadcrumbs inside of subdirectories of the contentPath (i.e. /content/notes).

Steps to reproduce

  1. Bootstrap a quick example with gatsby-theme-notes:
gatsby new my-themed-notes https://github.com/gatsbyjs/gatsby-starter-notes-theme

cd my-themed-notes

yarn develop

No custom configuration is needed, as the issue only appears under conditions where the user of gatsby-theme-notes has not provided a custom basePath option.

  1. In your browser, navigate to localhost:8000/example-dir.

Expected result

The breadcrumbs at the top of the page should look like this:

~ / example-dir

Actual result

The breadcrumbs at the top of the page look like this:

~ / / example-dir

Environment

❯ gatsby info --clipboard

  System:
    OS: macOS 10.14.5
    CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 11.14.0 - /usr/local/bin/node
    Yarn: 1.15.2 - /usr/local/bin/yarn
    npm: 6.9.2 - ~/.npm-packages/bin/npm
  Languages:
    Python: 2.7.16 - /usr/local/bin/python
  Browsers:
    Chrome: 77.0.3833.0
    Firefox: 65.0.1
    Safari: 12.1.1
  npmPackages:
    gatsby: ^2.13.6 => 2.13.6
    gatsby-theme-notes: ^1.0.0 => 1.0.0
  npmGlobalPackages:
    gatsby-cli: 2.7.3

Debugging

I’ve spent a little bit of time debugging the issue, and I’ve traced it back to 2 separate parts of the same file:

  1. Rendering the basePath text in the breadcrumb:
basePath.replace(/^\//, ``)
  1. Rendering the initial breadcrumb divider

In scenarios where basePath is the default value ”/“, the regex replacement in # 1 will render nothing. Then, in the component’s render method, the initial breadcrumb divider in # 2 is rendered.

The initial breadcrumb divider should only render if there is a custom basePath (or more importantly, if the basePath has a text length).

Unless this is the intended behavior, I can make a PR to fix. 😄

@johno
Copy link
Contributor

johno commented Jul 8, 2019

A PR to fix this would be great @jakewies!

@jakewies
Copy link
Contributor Author

jakewies commented Jul 8, 2019

Awesome I’ll get on that later today after the day job! 🤓

@jakewies
Copy link
Contributor Author

jakewies commented Jul 9, 2019

@johno is there any way I can test local changes to supported themes? I see some docs on contributing to core/plugins/docs/etc., but nothing related to official themes.

I can make my changes but would love to get some immediate feedback that they work. I guess I could do something like linking packages, but I’m curious if there is a supported way.

First time contributor 🤓

I’ve read some contribution docs and does mention gatsby-dev-cli. Is that the best route here?

@johno
Copy link
Contributor

johno commented Jul 9, 2019

@jakewies there is, apologies on it not being documented yet!

To run a given theme you can (from the root of the gatsby repo):

cd themes
yarn
yarn workspace gatsby-starter-theme start

For changes in gatsby-node you will likely need to restart the last command.

First time contributor 🤓

Welcome! 💜💜💜

I’ve read some contribution docs and does mention gatsby-dev-cli. Is that the best route here?

The themes side of things is new so our contributing docs aren't quite there yet. I'll spend some time tomorrow formalizing theme development docs. Thanks for your patience!

Let us know if you encounter any other issues, always happy to help.

@jakewies
Copy link
Contributor Author

jakewies commented Jul 9, 2019

@johno unfortunately still having some issues during development. Let me outline the process I took:

  1. Setup
cd themes
yarn
yarn workspace gatsby-starter-notes-theme start
  1. In gatsby-theme-notes I made the necessary changes to src/components/breadcrumbs.js.

  2. At this point I didn’t see any change in the browser, so I decided to re-compile and run the last command above.

yarn workspace gatsby-starter-notes-theme start
  1. Still no change 😢 . At this point I decided just to log something to the console and see if any of my code changes are getting to the browser. But unfortunately I got nothing.

I’m certain my code change works, as I’ve tested it in a local project that uses gatsby-theme-notes as a dependency, however I want to make sure it’s working in this PR. Any thoughts as to why I can’t see the change in development?

———

EDIT: Also, tried wiping cache in the gatsby-starter-notes-theme directory, but no result.

———

EDIT 2: I ran yarn workspaces info in the themes directory. Here’s the output:

❯ yarn workspaces info  

{
  "gatsby-theme-blog": {
    "location": "gatsby-theme-blog",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": []
  },
  "gatsby-theme-notes": {
    "location": "gatsby-theme-notes",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": []
  },
  "gatsby-starter-theme": {
    "location": "gatsby-starter-theme",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": [
      "gatsby-theme-blog",
      "gatsby-theme-notes"
    ]
  },
  "gatsby-starter-blog-theme": {
    "location": "gatsby-starter-blog-theme",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": [
      "gatsby-theme-blog"
    ]
  },
  "gatsby-starter-notes-theme": {
    "location": "gatsby-starter-notes-theme",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": [
      "gatsby-theme-notes"
    ]
  }
}

I’m not too familiar with yarn workspaces, but it looks like everything is labeled as a mismatchedWorkspaceDependency. Doesn’t sound positive 😆 And there doesn’t seem to be much documentation online about handling this potential issue (if it is one).

———

EDIT 3: Here’s a snapshot of themes/gatsby-starter-notes-theme/node_modules:

Screen Shot 2019-07-09 at 11 56 41 AM

It’s obvious to me now why my code changes aren’t showing in the browser, because gatsby-starter-notes-theme is depending on the gatsby-theme-notes inside of node_modules.

Why? I have no clue 😆 Obviously there is no symlinking going on. But I thought, with yarn workspaces, that this wouldn’t be an issue. Any ideas?

@marisamorby marisamorby added this to To prioritize in Themes Roadmap via automation Jul 11, 2019
@jakewies
Copy link
Contributor Author

Hey @johno not sure if you’ve had a chance to peak at my findings in the last week. No rush as this isn’t a pressing issue. Would love to help get this merged. I just need some guidance when developing these official themes. 😄

@lannonbr lannonbr changed the title gatsby-theme-notes is showing improper breadcrumbs when basePath is set to default value. [gatsby-theme-notes] showing improper breadcrumbs when basePath is set to default value. Jul 22, 2019
@johno
Copy link
Contributor

johno commented Aug 5, 2019

Hey @jakewies, thanks for your patience!

It looks like previously we had version mismatches between the themes and starters. gatsby-theme-notes should be correctly configured and gatsby-theme-blog will be when #16399 is merged.

@jakewies
Copy link
Contributor Author

Hey @johno, sorry for the delay. The fix has been made in #16556. Let me know if I can do anything else! 😄

Themes Roadmap automation moved this from To prioritize to Done Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants