[v2] fix find-page #5228
[v2] fix find-page #5228
Conversation
| if ( | ||
| typeof __PREFIX_PATHS__ !== `undefined` && | ||
| typeof __PATH_PREFIX__ !== `undefined` | ||
| ) { |
There was a problem hiding this comment.
Removing this if seems to make related tests unhappy ( https://travis-ci.org/gatsbyjs/gatsby/jobs/374034245#L1484 ) :(
There was a problem hiding this comment.
but we don't do this check in other places, I just want to make them consistency, what about change the test code here https://github.com/gatsbyjs/gatsby/blob/v2/packages/gatsby/cache-dir/__tests__/loader.js#L5-L12?
There was a problem hiding this comment.
We should reset them to the proper values like '' and false instead of undefined, also I think we can drop the workaround for Node v6, do we still support that node version?
I see we still support v6 in the package.json, I think we can drop the support for that
There was a problem hiding this comment.
Yeah, we can adjust tests, sure. Just keep in mind that we should check tests (some of them are still failing on v2 but we are working to get them all of the passing again - just let's not break tests that are already passing - either adjust code or tests - whichever make sense)
We want to support node 6 (related comment chain - #2641 (comment) ).
There was a problem hiding this comment.
Thanks for clarifying
There was a problem hiding this comment.
tests are passing now
| } | ||
| findPage = pageFinderFactory(newPages, pathPrefix) | ||
| findPage = pageFinderFactory(newPages, pathPrefix.slice(0, -1)) |
There was a problem hiding this comment.
Why not just use __PATH_PREFIX__ here? The slicing is confusing.
There was a problem hiding this comment.
because we use slice in other places, when calculating then basename, I just want to keep consistency, I agree it's a bit redundant, but I think we can do it in another pathPrefix refactor which I described in the issues's description
There was a problem hiding this comment.
ah right :-) You did cover this in the description 👍to that
right now find-page is broken with
prefix-pathsBTW, I think it's confusing to add the trailing slash to the
prefixPathfrom user definition, but still use the same var name, it should bestripBasenamefrom my understanding,so about about rename the
prefix-pathstobasenamein thegatsby-configsection, thenbasenamewith trailing slash is pathPrefix, and we can address it in another PR, any thoughts @KyleAMathews @m-allanson @jquense