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

RFC: -t for remote codemods #94

Closed
iamdustan opened this issue Mar 8, 2016 · 13 comments
Closed

RFC: -t for remote codemods #94

iamdustan opened this issue Mar 8, 2016 · 13 comments

Comments

@iamdustan
Copy link
Contributor

Currently, community-specific codemods are appearing inside specific repos without much shareability and the ergonomics to apply them require quite a bit of manual effort (especially for a tool based around removing manual effort).

Allowing -t to accept remote codemods would enable a generic way to build a hosted service and share codemods.

iamdustan added a commit to iamdustan/jscodeshift that referenced this issue Mar 8, 2016
@fkling
Copy link
Contributor

fkling commented Mar 8, 2016

👍 I was wondering how you imagined this to work until I saw your PR ;) Yeah, one drawback is that scripts need to be self contained.

I'm not really fond of the current practice either. Being able to load codemods remotely is definitely a good idea. We could expand that and make it easier to load them from GitHub or gists.

If someone wants to share a more complex codemod with dependencies, they could just put it on npm.

@iamdustan iamdustan reopened this Mar 8, 2016
@iamdustan
Copy link
Contributor Author

The Browserify web service (https://wzrd.in/) could likely be used ad-hoc for the first constraint (h/t @RReverser)

Ensuring the transform is written in valid node code and not babel is the second constraint (e.g React-router codemods use es6 modules)

@fkling
Copy link
Contributor

fkling commented Mar 8, 2016

Ensuring the transform is written in valid node code and not babel is the second constraint

jscodeshift passes transforms through Babel by default. But I guess that won't work with the browserify web service then.

@iamdustan
Copy link
Contributor Author

What areas would you like me to explore and verify before moving forward with #95?

iamdustan added a commit to iamdustan/jscodeshift that referenced this issue Mar 11, 2016
iamdustan added a commit to iamdustan/jscodeshift that referenced this issue Mar 11, 2016
@fkling
Copy link
Contributor

fkling commented Mar 11, 2016

I'm fine with it as it is (if the test passes). We can always extend this when there is demand.

@iamdustan
Copy link
Contributor Author

A test broke when remerging with master. Hope to get that fixed today.

iamdustan added a commit to iamdustan/jscodeshift that referenced this issue Mar 11, 2016
@iamdustan
Copy link
Contributor Author

Found the reason. #95 was changed from 2 to 0 in the diff.

@fkling fkling closed this as completed in 027cd02 Mar 14, 2016
@cpojer
Copy link
Contributor

cpojer commented Mar 14, 2016

NICE!

@iamdustan
Copy link
Contributor Author

Who wants to add friendly URLs and the ability to request just the transform to astexplorer? 😀

@fkling
Copy link
Contributor

fkling commented Mar 15, 2016

astexplorer support is going to be more tricky, since the data has to be loaded from Parse somehow. But Parse is shutting down and we don't know yet where to move it to.

If we would run a VPS, we can have the server expose an API to load astexplorer transform data.

@RReverser
Copy link

@fkling I just thought about that once again - why don't we move to Gists instead as some popular JS playgrounds do? That way, user could load, save and share actual gists across the playgrounds and we wouldn't need to care about the storage at all.

@fkling
Copy link
Contributor

fkling commented Mar 15, 2016

@RReverser: I'd like that, I just haven't used gists programmatically yet.

@RReverser
Copy link

@fkling My only concern is that all the old URLs will still break. Not sure how to deal with them in that case.

euphocat pushed a commit to euphocat/jscodeshift that referenced this issue Oct 22, 2017
* Add description of `pure-component` transform

* Basic destructuring works

Needs to be tested
Only works if there are no references to `this.props` by itself
Would be nice if it would work for (replace) in-render destructuring

* Remove duplicate variable declarations

If `bar` is assigned to `this.props.bar` we need to remove that, as it's available as a destrctured arg

* Test various scenarios

* Update readme with destructure option

* Undo accidental indentation change

* Made propNames be a set to make sure they are unique

* Type Arrays to Array

* Rename option to `destructuring`

* Simplify build argument

* Identify shadowing and do not destructure

* Retain type annotations

* Change option name in readme

destructure -> destructuring
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

No branches or pull requests

4 participants