Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add a navigateTo -> navigate codemod #7532

Merged
merged 7 commits into from
Sep 4, 2018

Conversation

DSchau
Copy link
Contributor

@DSchau DSchau commented Aug 22, 2018

This PR adds a (probably at least somewhat naive) codemod for migrating from navigateTo to navigate. I think there's still more to do here, but this should at least be a good start.

Additionally, I fixed the commented out test in the other, existing codemod.

Consider this kind of a WIP, I want to write a few more tests before I'd be comfortable with other people actually using this thing.

DSchau added 4 commits August 21, 2018 22:19
Note: this makes an assumption that we support ESM in the absence of any require or import statements... we _have_ to make an assumption though, so this seems like at least a decent fallback
@@ -32,14 +32,18 @@ function addEsmImport(j, root, tag) {
return // already exists

if (!existingImport.length) {
const first = root.find(j.Program).get('body', 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may not actually need this because there will likely be a import React from 'react'; or const React = require('react'); as the first line, correct? If that's the case, I can tweak this a bit and just use insertAfter

@@ -119,7 +123,12 @@ module.exports = (file, api, options) => {
let tag = tags.get(0)

const useImportSyntax =
root.find(j.ImportDeclaration, { importKind: `value` }).length > 0
root.find(j.ImportDeclaration, { importKind: `value` }).length > 0 ||
root.find(j.VariableDeclarator, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an assumption. If there isn't any import or require statement, we have to make a decision here. I think we should prefer esm, but I'd rather not make any assumption--thus see above where I think we'll always have at least one import/require statement in the file

Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

Thanks @DSchau, this looks great 👍 How do you feel about merging it in so people can try it out?

import React from "react"
import { navigate } from 'gatsby';

// Don't use navigate with an onClick btw :-)
Copy link
Contributor

Choose a reason for hiding this comment

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

😄

@DSchau DSchau merged commit 5f679fa into gatsbyjs:master Sep 4, 2018
@DSchau DSchau deleted the codemods/navigate branch September 4, 2018 13:25
@m-allanson m-allanson mentioned this pull request Sep 4, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants