Skip to content

Lift requires #3780

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.

Sign up for GitHub

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

Merged
cpojer merged 6 commits into from
Jun 10, 2017
Merged

Lift requires #3780

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c62816
Lift all inline requires to the top-level
SimenB Jun 9, 2017
8662fa1
Remove one inlining that messes us up
SimenB Jun 9, 2017
3c45c08
Inline two more requires causing the repl test to fail
SimenB Jun 9, 2017
bc2c147
remove one unnecessary inline
SimenB Jun 9, 2017
b33d5f4
Fix prettier lint
SimenB Jun 9, 2017
69095fe
really prettier?
SimenB Jun 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/jest-resolve/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ class Resolver {

static findNodeModule(path: Path, options: FindNodeModuleConfig): ?Path {
const resolver = options.resolver
/* $FlowFixMe */
? require(options.resolver)
? /* $FlowFixMe */
Copy link
Member Author

@SimenB SimenB Jun 9, 2017

Choose a reason for hiding this comment

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

Not a fan of this one, /cc @vjeux

Copy link
Contributor

@vjeux vjeux Jun 9, 2017

Choose a reason for hiding this comment

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

I'm curious, is this a regression from before?

It indeed looked better before.

Copy link
Member Author

@SimenB SimenB Jun 9, 2017

Choose a reason for hiding this comment

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

No, it was in a single line before, it was multilined when I made it into a ternary

Copy link
Contributor

@vjeux vjeux Jun 9, 2017

Choose a reason for hiding this comment

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

Could you open an issue against prettier? We may be able to print it the way you want, we're already doing that for flow unions.

Copy link
Member Author

@SimenB SimenB Jun 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Will do!

Copy link
Member Author

@SimenB SimenB Jun 9, 2017

Choose a reason for hiding this comment

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

prettier/prettier#2076

require(options.resolver)
: defaultResolver;
const paths = options.paths;

Expand Down