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] Use more optimal suffix-set format #11784

Conversation

EvanBacon
Copy link
Contributor

Summary

While looking through the code (expo-cli-(react-native)->metro->jest), I noticed that the syntax used here is specifically advised against in the watchman docs. I don't know enough about watchman to determine if these docs are incorrect, but @cpojer recommended opening a PR.

Test plan

Running the command locally works:

$ watchman -j <<-EOT                                                                        
["query", "./", {           
  "expression": ["allof",                
    ["type", "f"],      
    ["suffix", ["js", "json", "ts"]]
  ],                
  "fields": ["name", "exists", "mtime_ms", "size"]
}] 
EOT

@SimenB
Copy link
Member

SimenB commented Aug 25, 2021

Should update the test and fix linting error.

That said, I know nothing of watchman 😅 @rickhanlonii is this something you can verify with somebody at FB?

@motiz88
Copy link
Contributor

motiz88 commented Aug 25, 2021

Looks like jest-haste-map already does some capability negotiation with Watchman:

https://github.com/facebook/jest/blob/bc50e7f360ab1845abbaa0b3ad788caead0d3174/packages/jest-haste-map/src/watchers/WatchmanWatcher.js#L175-L180

So for compatibility with older Watchman versions, let's check for the suffix-set capability first and otherwise fall back to the old behaviour.

Other than that, looks good!

@SimenB
Copy link
Member

SimenB commented Aug 25, 2021

Thanks @motiz88!

@SimenB
Copy link
Member

SimenB commented Aug 25, 2021

Changelog entry and updated tests and I think we're good to go 🙂

@SimenB SimenB merged commit d455d2d into jestjs:master Aug 27, 2021
@EvanBacon EvanBacon deleted the @evanbacon/jest-haste-map/update-watchman-crawler-request branch August 28, 2021 03:50
@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 Sep 28, 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.

None yet

4 participants