-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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-remark-copy-linked-files] Fix bad copy location with route dir and improve tests #2875
[gatsby-remark-copy-linked-files] Fix bad copy location with route dir and improve tests #2875
Conversation
Deploy preview ready! Built with commit 861224c |
Deploy preview ready! Built with commit 861224c |
🥇 for PR description of the day |
Suppressed how? I use console.log all the time in plugins during development. |
The tests are failing on Windows https://ci.appveyor.com/project/KyleAMathews/gatsby/build/1.0.3942/job/39cjovk10p1emxcn I think you need to use normalize-path c.f. #2658 (comment) to ensure paths are the same in all OSes. |
Struggling to find the issue for some reason but I wonder if this is related to a bug @bvaughn found on reactjs.org where |
There were a couple of |
I actually fixed a bug like this - it was caused by passing the actual opts object into the plugin (instead of a copy). The opts object was mutated, and the mutated object became the new opts object. I was going to suggest that you pass in copies to the plugins as this is much safer. |
No path.join is correct — you just need to use normalize-path. If you read the comment I linked to, using path.posix.join was the cause of previous windows breakage. |
Oh weird… yeah I guess we shouldn't trust plugins to not mutate options :-( Want to add a PR to clone the options here?
|
Looks like the tests for
Sure. |
I'm confused - the comment you link to says:
And the plugin already used And the failing tests are not tests for Sorry if I'm being dense. Long day. |
Oh hmm sorry — I'm being dense :-) Yeah you're right — since posix.join is fine in this case as you're just generating a path for the test from hard coded values — not from paths you read from the OS which would vary by OS. Ok, merging this — there are some other windows test failures which have crept in which I'll look into. |
The ongoing saga of adding destinationDir option to gatsby-copy-linked-files The third in a trilogy, but unlike Robocop 3, this is the best of the bunch.
@KyleAMathews Using the gatsby-dev-cli has been really helpful. Not sure how I missed it before, but I've noticed that all logging within plugins (when run in an app/site) is suppressed. Is there a way to enable logging in plugins when running them? I mean that logging works fine when testing within the Gatsby project, but when I pull the plugin into a project that uses Gatsby, all logging is gone. Even if I directly add to the derived file in
node_modules
.