diegohaz / generact

Generate React components by replicating your own
https://git.io/generact
MIT License
1.48k stars 64 forks source link

Feature: renaming imports from current folder #14

Closed alvelig closed 6 years ago

alvelig commented 7 years ago

Feature proposal:

rename imports from './{sourceComponentName}' to './{destinationComponentName}'

diegohaz commented 7 years ago

I would love to have this feature, since I use generact a lot for just renaming components (because naming things is hard).

But that conflicts with the default usage of generact (creating new components based on the existing ones). In this case, we don't want to rename imports.

I don't know how to solve it. :(

alvelig commented 7 years ago

I did not understand the conflict, for me the naming convention is simple: App - component folder

So for rename import two conditions should be met:

  1. current folder import (from './')
  2. import starts with old component name (App)

So, if it's ./App*.js rename App to the new component name.

Anything else remains untouched.

What's your case?

diegohaz commented 7 years ago

Ah, I see. I was talking about a completely different thing. Nevermind.

Currently, we've restricted it to not change AnythingAppElse names when replacing file contents (here: https://github.com/diegohaz/generact/blob/master/src/utils.js#L65). That's because AnythingAppElse could be an external component, which we don't want to touch.

But, as you stated, we can safely do that when it's preceded by ./. Changing that function should be enough. Would you want to work on that?

alvelig commented 7 years ago

Let me try :) Seems like a not large bit of work.

diegohaz commented 7 years ago

Nice. :)

You can start by updating tests for what you expect to have https://github.com/diegohaz/generact/blob/master/test/utils.test.js#L82-L96

Tell me if you need any help

alvelig commented 7 years ago

Do you think these are valid conditions?

test('replaceContents', () => {
  const contents = `
    import './Button.css'
    import ButtonComponent from '../ButtonComponent'
    import NodeButtonComponent from 'node-button-component'
    import NodeButtonComponent from 'node-component/Button'
    import SimpleButton from './SimpleButton'
    import { someButtonUtil } from './SimpleButtonUtils'
    const Button = () => <button />
    export const someButtonUtil = () => {}
    export default Button
  `

  expect(replaceContents(contents, 'Button', 'AnotherButton')).toBe(`
    import './AnotherButton.css'
    import ButtonComponent from '../ButtonComponent'
    import NodeButtonComponent from 'node-button-component'
    import NodeButtonComponent from 'node-component/Button'
    import SimpleAnotherButton from './SimpleAnotherButton'
    import { someAnotherButtonUtil } from './SimpleAnotherButtonUtils'
    const AnotherButton = () => <button />
    export const someAnotherButtonUtil = () => {}
    export default AnotherButton
  `)
})

Updated.

diegohaz commented 7 years ago

Yeah, that seems pretty good.

alvelig commented 7 years ago
import { someButtonUtil } from './SimpleAnotherButtonUtils'

someButtonUtil should be renamed as well :( Because export const someButtonUtil will be renamed, as far as I see.

I'll start PR, but still can't figure a RegExp to match that... That's not my strong part 😕

diegohaz commented 7 years ago

I think someButtonUtil should remain as it is. Otherwise, things like NodeButtonComponent would be changed as well.

Update: It will be hard to deal with SimpleAnotherButton references too. :(

alvelig commented 7 years ago

Updated version:

test('replaceContents', () => {
  const contents = `
    import './Button.css'
    import ButtonComponent from '../ButtonComponent'
    import NodeButtonComponent from 'node-button-component'
    import NodeButtonComponent from 'node-component/lib/Button'
    import NodeYellowButtonComponent from 'node-component/YellowButton'
    import SimpleButton from './SimpleButton'
    import { someButtonUtil } from './SimpleButtonUtils'
    const Button = () => <button />
    export const someButtonUtil = () => {}
    export default Button
  `

  expect(replaceContents(contents, 'Button', 'AnotherButton')).toBe(`
    import './AnotherButton.css'
    import ButtonComponent from '../ButtonComponent'
    import NodeButtonComponent from 'node-button-component'
    import NodeButtonComponent from 'node-component/lib/Button'
    import NodeYellowButtonComponent from 'node-component/YellowButton'
    import SimpleAnotherButton from './SimpleAnotherButton'
    import { someButtonUtil } from './SimpleAnotherButtonUtils'
    const AnotherButton = () => <button />
    export const someButtonUtil = () => {}
    export default AnotherButton
  `)
})
alvelig commented 7 years ago

Actually this one is changing and it shouldn't:

    -    import NodeButtonComponent from 'node-component/lib/Button'
    +    import NodeButtonComponent from 'node-component/lib/AnotherButton'
alvelig commented 6 years ago

What if we made a kind of setting to add extra RegExp for users who want to customize replaceContents per project?

.generact or something like that