Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Update to latest jscodeshift #17

Merged
merged 1 commit into from
Jun 14, 2017

Conversation

xixixao
Copy link
Contributor

@xixixao xixixao commented Jun 14, 2017

By updating to latest jscodeshift:

  1. This fixes the iife test error that was caused by a bug in recast, fixed in: Don't wrap IIFEs in parens when using babylon benjamn/recast#410
  2. This fixes support for bare function arguments, which were breaking recast due to a bug in babylon, fixed in: Fix location info on FunctionTypeParam nodes babel/babylon#565 and adds a test for the corresponding syntax

I have also removed dead dependencies from package.json. Note why the remaining dep is not locked down, explained to me by @bestander:

using 1.2.3 instead of ^1.2.3 does not give you stability actually, in many cases it is actually worse 🙂
The problem is that ^ is the default for npm and all the packages that you depend on use ^.
So left-pad@1.2.3 will download the same left-pad but its dependencies are specified as the author of left-pad did, with ^.
So you'll have variance in subdependencies between runs.
That is why yarn.lock is needed.
BTW, don't worry about npm, they use their own lock file by default too, now.

Before we release all the updates, I want do two last things (ideally tomorrow):

  1. Make sure the sorting is correct for both types and modules with our order-requires rule
  2. Add a notification when the formatter fails due to a syntax error in user's source code, instead of silently failing

Plus:

  • Compare perf with Flow parser
  • Document use in codemods

@kyldvs
Copy link
Contributor

kyldvs commented Jun 14, 2017

Having it locked down was previously required for Nuclide in some fashion, I'm not aware of if it's still necessary since this is external, but to be on the safe side that's why I requested to keep them locked down to a specific version.

@kyldvs kyldvs merged commit 8522280 into facebookarchive:master Jun 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants