Lift requires #3780

SimenB · 2017-06-09T15:44:29Z

Summary
Move a lot of the changes in #3493 to isolate the changes causing errors.

Test plan
Tests should pass

We want to inline them using a babel plugin

SimenB · 2017-06-09T15:45:20Z

This fails locally for some reason.

 FAIL  packages/jest-repl/src/__tests__/jest-repl-test.js                                                                                                                                                                                [2/2]
  ● Repl › cli › runs without errors

    expect(received).toMatch(expected)

    Expected value to match:
      /›/
    Received:
      ""

      at Object.it (packages/jest-repl/src/__tests__/jest-repl-test.js:28:36)
          at Promise (<anonymous>)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:169:7)

  Repl
    cli
      ✕ runs without errors (513ms)

SimenB · 2017-06-09T15:52:54Z

packages/jest-cli/src/cli/getJest.js

@@ -21,6 +21,7 @@ function getJest(packageRoot: Path) {
    /* $FlowFixMe */
    return require(binPath);
  } else {
+    // TODO: Because of a dependency cycle, this can only be inlined once the babel plugin for inlining is merged


This should almost be considered a bug, IMO

SimenB · 2017-06-09T16:11:04Z

packages/jest-resolve/src/index.js

@@ -91,8 +91,8 @@ class Resolver {

  static findNodeModule(path: Path, options: FindNodeModuleConfig): ?Path {
    const resolver = options.resolver
-      /* $FlowFixMe */
-      ? require(options.resolver)
+      ? /* $FlowFixMe */


Not a fan of this one, /cc @vjeux

https://prettier.github.io/prettier/#%7B%22content%22%3A%22const%20resolver%20%3D%20options.resolver%5Cn%20%20%2F*%20%24FlowFixMe%20*%2F%5Cn%20%20%3F%20require(options.resolver)%5Cn%20%20%3A%20defaultResolver%3B%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3A%22none%22%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22parser%22%3A%22babylon%22%2C%22semi%22%3Atrue%2C%22useTabs%22%3Afalse%2C%22doc%22%3Afalse%7D%7D

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

It indeed looked better before.

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

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.

prettier/prettier#2076

codecov-io · 2017-06-09T16:44:35Z

Codecov Report

Merging #3780 into master will decrease coverage by 0.07%.
The diff coverage is 52.63%.

@@            Coverage Diff             @@
##           master    #3780      +/-   ##
==========================================
- Coverage   58.09%   58.02%   -0.08%     
==========================================
  Files         191      191              
  Lines        6708     6711       +3     
  Branches        6        6              
==========================================
- Hits         3897     3894       -3     
- Misses       2808     2814       +6     
  Partials        3        3

Impacted Files	Coverage Δ
packages/jest-cli/src/TestNamePatternPrompt.js	`94.11% <ø> (ø)`	⬆️
...ages/pretty-format/src/plugins/ImmutablePlugins.js	`100% <ø> (ø)`	⬆️
...kages/pretty-format/src/plugins/ImmutableRecord.js	`100% <ø> (ø)`	⬆️
packages/jest-validate/src/utils.js	`100% <ø> (ø)`	⬆️
packages/eslint-config-fb-strict/index.js	`0% <ø> (ø)`	⬆️
packages/jest-cli/src/TestRunner.js	`25.3% <ø> (+0.15%)`	⬆️
packages/eslint-plugin-jest/src/index.js	`100% <ø> (ø)`	⬆️
packages/pretty-format/src/index.js	`98.05% <ø> (ø)`	⬆️
packages/jest-changed-files/src/index.js	`0% <0%> (ø)`	⬆️
packages/jest-validate/src/index.js	`0% <0%> (ø)`	⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dec54a3...69095fe. Read the comment docs.

* Lift all inline requires to the top-level We want to inline them using a babel plugin * Remove one inlining that messes us up * Inline two more requires causing the repl test to fail * remove one unnecessary inline * Fix prettier lint * really prettier?

github-actions · 2021-05-13T13:13:48Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

SimenB added 2 commits June 9, 2017 17:27

Lift all inline requires to the top-level

5c62816

We want to inline them using a babel plugin

Remove one inlining that messes us up

8662fa1

facebook-github-bot added the CLA Signed ✔️ label Jun 9, 2017

Inline two more requires causing the repl test to fail

3c45c08

SimenB commented Jun 9, 2017

View reviewed changes

SimenB mentioned this pull request Jun 9, 2017

Inline require #3493

Closed

SimenB added 2 commits June 9, 2017 17:59

remove one unnecessary inline

bc2c147

Fix prettier lint

b33d5f4

SimenB force-pushed the lift-requires branch from 407d92d to b33d5f4 Compare June 9, 2017 16:07

really prettier?

69095fe

SimenB commented Jun 9, 2017

View reviewed changes

SimenB mentioned this pull request Jun 9, 2017

Comment inside multiline ternary moves after ? prettier/prettier#2076

Open

SimenB added a commit to SimenB/jest that referenced this pull request Jun 9, 2017

[squash] package up jestjs#3780

56858d6

cpojer merged commit e0e170a into jestjs:master Jun 10, 2017

SimenB deleted the lift-requires branch June 10, 2017 16:12

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lift requires #3780

Lift requires #3780

SimenB commented Jun 9, 2017

SimenB commented Jun 9, 2017

SimenB Jun 9, 2017

SimenB Jun 9, 2017

SimenB Jun 9, 2017

vjeux Jun 9, 2017

SimenB Jun 9, 2017

vjeux Jun 9, 2017

SimenB Jun 9, 2017 •

edited

Loading

SimenB Jun 9, 2017

codecov-io commented Jun 9, 2017

github-actions bot commented May 13, 2021

Lift requires #3780

Lift requires #3780

Conversation

SimenB commented Jun 9, 2017

SimenB commented Jun 9, 2017

SimenB Jun 9, 2017

Choose a reason for hiding this comment

SimenB Jun 9, 2017

Choose a reason for hiding this comment

SimenB Jun 9, 2017

Choose a reason for hiding this comment

vjeux Jun 9, 2017

Choose a reason for hiding this comment

SimenB Jun 9, 2017

Choose a reason for hiding this comment

vjeux Jun 9, 2017

Choose a reason for hiding this comment

SimenB Jun 9, 2017 • edited Loading

Choose a reason for hiding this comment

SimenB Jun 9, 2017

Choose a reason for hiding this comment

codecov-io commented Jun 9, 2017

Codecov Report

github-actions bot commented May 13, 2021

SimenB Jun 9, 2017 •

edited

Loading