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

Warn on all runtime fs.sync* calls that need to be converted to be async and convert to async #22028

Closed
sync-by-unito bot opened this issue Jun 2, 2022 · 0 comments

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Jun 2, 2022

Summary

We need to ensure we’re not using synchronous calls that open file descriptors (fs.readFile ) as much as possible, since they block the main thread. Instead use async FS functions.

Currently we lint sync FS methods: #22175

But this doesn't reach into node_modules that use sync methods, and is not a runtime check.

Acceptance Criteria

  • During development, should add a warning when fs.readFileSync / fs.readdirSync / globby.sync
    • Or throw an error - error might be better so warnings dont make it to develop
    • Add a monkey patch for both fs + fs-extra with the warning, so we’re consistent. Ensure we’re only doing so for development, since there’s nothing a user could do if we somehow missed one
    • We should add the monkey-patched warnings in a nextTick, since we’re considering it valid to do sync file reads at the top level of a file for now, though eventually we can refactor and/or move to top-level-await with es modules
  • Fix remaining sync FS method usage

Resources

@flotwig began spiking into the warnings here: #22176

┆Issue is synchronized with this Jira Task by Unito
┆author: Brian Mann
┆friendlyId: UNIFY-1770
┆priority: High
┆sprint: Fast Follows 1
┆taskType: Task

@cypress-bot cypress-bot bot added stage: to do stage: needs review The PR code is done & tested, needs review and removed stage: work in progress stage: needs review The PR code is done & tested, needs review labels Jun 8, 2022
@flotwig flotwig changed the title Audit: surface / annotate all runtime fs.sync* calls that need to be converted to be async Warn on all runtime fs.sync* calls that need to be converted to be async Jun 21, 2022
@flotwig flotwig removed their assignment Jun 21, 2022
@flotwig flotwig changed the title Warn on all runtime fs.sync* calls that need to be converted to be async Warn on all runtime fs.sync* calls that need to be converted to be async and convert to async Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants