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

fix(gatsby-source-wordpress): image fixes #29813

Merged
merged 5 commits into from
Mar 1, 2021
Merged

Conversation

TylerBarnes
Copy link
Contributor

@TylerBarnes TylerBarnes commented Feb 27, 2021

This is a collection of 3 quick/emergency fixes.
I was debugging a customer site and found that in their case these 3 things were broken (all related to transforming images in html or file connection fields). This is a single PR because these all need to be fixed for that feature to work.

1a61ee7 and a9b139c are permanent fixes.

4226459 isn't great and has a bit of a back-story. Early on in this plugins development I copied over create-remote-file-node to the WP plugin to debug some things - I ended up changing it quite a bit and never PR'ed the fixes back to core (bad monkey! 🙈). Well my copied version is importing an export from gatsby-source-filesystem that no longer exists.. which isn't great. This is a temporary fix and the copied create-remote-file-node will be removed soon - in the meantime we need to fix it like this.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 27, 2021
@TylerBarnes TylerBarnes removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 27, 2021
@TylerBarnes TylerBarnes changed the title fix(gatsby-source-wordpress): html image fixes fix(gatsby-source-wordpress): image fixes Feb 28, 2021
normalizedMatch + `(?!/?wp-content|/?wp-admin|/?wp-includes)`,
`g`
)

nodeString = nodeString.replace(thisMatchRegex, normalizedPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always helpful to add a comment with an example of what this converts into

Comment on lines +142 to +143
console.error(error)
reporter.panic()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with panicking with an error?

Suggested change
console.error(error)
reporter.panic()
reporter.panic(error)

Copy link
Contributor Author

@TylerBarnes TylerBarnes Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reporter.panic() is broken right now - this will result in Gatsby logging "There was an error" and quitting with no additional info https://gatsby.canny.io/ideas/p/reporterpanic-is-broken

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TylerBarnes I published it in gatsby-source-wordpress@wp-fixes

The cherry-pick into v2 will need the createProgressbar fix but that needs to be done in the cherry-pick PR and not in the master PR.

@TylerBarnes
Copy link
Contributor Author

Thanks @wardpeet and @sslotsky !

@TylerBarnes TylerBarnes merged commit 28124dd into master Mar 1, 2021
@TylerBarnes TylerBarnes deleted the fix/emergency-wp-fixes branch March 1, 2021 15:46
ascorbic pushed a commit that referenced this pull request Mar 1, 2021
Co-authored-by: Ward Peeters <[email protected]>
(cherry picked from commit 28124dd)
vladar pushed a commit that referenced this pull request Mar 1, 2021
Co-authored-by: Ward Peeters <[email protected]>
(cherry picked from commit 28124dd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants