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

jest-haste-map: fix bug where platform-specific files are removed #5534

Merged
merged 2 commits into from
Feb 12, 2018

Conversation

jeanlauliac
Copy link
Contributor

Another fix for jest-haste-map 😁
This changeset fixes the bug that was reproduced by the test I added a few days ago, where platform-specific file removals aren't taken into account. I traced this back to the fact the logic just copies the entire module metadata for a particular Haste name, without discriminating between platforms. That means if any of the files for that Haste name is still here, all the platforms already cached are included in the new map, even if some of these files were deleted. With the new logic, we only copy over the data for the platform of the file being processed.

I didn't check how that behaves for the watch mode but I suspect this fixes the bug for it as well. I can follow up with another test for that.

@jeanlauliac jeanlauliac force-pushed the fix-platform-specific-files-removal branch from fc0971c to c6055a9 Compare February 12, 2018 14:57
@rickhanlonii
Copy link
Member

Great stuff, missing a changelog 😄

@jeanlauliac
Copy link
Contributor Author

jeanlauliac commented Feb 12, 2018

The linter (yarn lint --fix) changed a completely unrelated file in the website folder, I assume listing was broken on master .

I added the line to the changelog. I've got bad habits ahah, I already made a few fixes before that weren't added to the changelog.

@codecov-io
Copy link

codecov-io commented Feb 12, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@508f789). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5534   +/-   ##
=========================================
  Coverage          ?   61.71%           
=========================================
  Files             ?      213           
  Lines             ?     7149           
  Branches          ?        4           
=========================================
  Hits              ?     4412           
  Misses            ?     2736           
  Partials          ?        1
Impacted Files Coverage Δ
packages/jest-haste-map/src/index.js 96.19% <100%> (ø)

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 508f789...499e1e0. Read the comment docs.

@cpojer
Copy link
Member

cpojer commented Feb 12, 2018

Nice, thanks for fixing this.

Re the prettier issue: I wanted to fix it this morning but got carried away and forgot.

@SimenB SimenB deleted the fix-platform-specific-files-removal branch February 12, 2018 16:14
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants