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

feat(gatsby): Remove old touchNode signature #29245

Merged
merged 9 commits into from
Feb 10, 2021
Merged

Conversation

LekoArts
Copy link
Contributor

@LekoArts LekoArts commented Jan 28, 2021

Description

Similarly to #29205 this removes an old arg signature, namely calling touchNode with a string (the id). This PR also adds a new deprecation so that in the future touchNode will be called like createNode and deleteNode -- with touchNode(node).

Documentation

The docs (source plugin guide) are updated.

Related Issues

[ch23714]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jan 28, 2021
@LekoArts LekoArts added topic: internal topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jan 28, 2021
@LekoArts LekoArts marked this pull request as ready for review February 2, 2021 10:05
gillkyle
gillkyle previously approved these changes Feb 8, 2021
Copy link
Contributor

@gillkyle gillkyle left a comment

Choose a reason for hiding this comment

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

This looks good to me, the warning looks helpful too!

I left one small suggestion that I'm not even sure makes a difference is all 🙂

packages/gatsby/src/redux/actions/public.js Outdated Show resolved Hide resolved
Co-authored-by: Kyle Gill <kylerobertgill@gmail.com>
@LekoArts
Copy link
Contributor Author

LekoArts commented Feb 9, 2021

@TylerBarnes I also had to update the WordPress plugin (will do the same in #29205)

@TylerBarnes
Copy link
Contributor

@LekoArts does this depend on a newer version of Gatsby? I haven't PR'd over the tests from the WP experimental repo yet so I'm a bit nervous to make changes to it.

@LekoArts
Copy link
Contributor Author

@TylerBarnes The change is made only here in this PR, so how about you merging your tests, I rebase onto master and we'll see if they work?

@wardpeet wardpeet changed the title gatsby: Remove old touchNode signature feat(gatsby): Remove old touchNode signature Feb 10, 2021
@LekoArts
Copy link
Contributor Author

@TylerBarnes To get myself unblocked and you not urged to merge your tests ASAP I reverted the WordPress changes :) The current state is now using deprecated APIs then, but it still works.

@LekoArts LekoArts merged commit 0927cb0 into master Feb 10, 2021
@LekoArts LekoArts deleted the v3/touch-node-nodeid branch February 10, 2021 11:46
benabel added a commit to gatsby-contrib/gatsby-plugin-elasticlunr-search that referenced this pull request Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants