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

scanImports() function reads all files at once, can cause EMFILE in larger projects #1245

Closed
FredKSchott opened this issue Oct 7, 2020 · 4 comments
Assignees
Labels
bug Something isn't working contributors welcome! contributors welcome! good first issue Good for newcomers

Comments

@FredKSchott
Copy link
Owner

FredKSchott commented Oct 7, 2020

Original Discussion: #1244
/cc @N8-B

The scanner appears to be causing trouble in the scanImports() function, where we call fs.promises.readFile() on all scanned files at once. You appear to be the first to hit this limit in a breaking fashion! (others at your size are probably just seeing degraded performance).

We use p-queue in the site builder for this exact reason, we should bring that same library into scanImports() to make sure that we are never loading more than X files at once in parallel. Node.js is great at I/O, so I think this number can be pretty high.

This is a great issue for anyone to tackle since it doesn't rely on any deeper understanding of Snowpack! Also should be covered by existing tests (a test directory with 10k files would be great to have, but probably out of scope for now).

@FredKSchott FredKSchott added bug Something isn't working contributors welcome! contributors welcome! good first issue Good for newcomers labels Oct 7, 2020
@N8-B
Copy link
Contributor

N8-B commented Oct 8, 2020

Thanks for opening up this issue. I'll look into it today and run tests on our code base and deploy environment.

N8-B added a commit to N8-B/snowpack that referenced this issue Oct 8, 2020
…arger projects FredKSchott#1245

See FredKSchott#1245
Adds 'p-queue' dependency
Creates `getLoadedFiles` mapper function to improve type legibility
FredKSchott pushed a commit that referenced this issue Oct 10, 2020
…arger projects #1245 (#1256)

* Recovers webpack magic comments and adds to import

Webpack's magic comments must be extracted from dynamic imports before the spec is passed to `matchDynamicImportValue`. If a match is found for these types of comments, they are appended to the rewritten import.

* scanImports() function reads all files at once, can cause EMFILE in larger projects #1245

See #1245
Adds 'p-queue' dependency
Creates `getLoadedFiles` mapper function to improve type legibility

* Bumps CONCURRENT_FILE_READS to 1000 and removes onIdle call #1256
@FredKSchott
Copy link
Owner Author

Fixed by #1256

@alexeychikk
Copy link

I am still having this issue:
EMFILE: too many open files, open ...
"snowpack": "^2.18.4"
My config:

/** @type {import("snowpack").SnowpackUserConfig } */
module.exports = {
  mount: {
    public: { url: '/', static: true },
    src: { url: '/dist' },
  },
  plugins: [
    '@snowpack/plugin-webpack',
    '@snowpack/plugin-react-refresh',
    '@snowpack/plugin-dotenv',
    '@snowpack/plugin-typescript',
  ],
  install: [
    /* ... */
  ],
  installOptions: {
    namedExports: ['react-google-login'],
  },
  devOptions: {
    /* ... */
  },
  buildOptions: {
    /* ... */
  },
  proxy: {
    /* ... */
  },
  alias: {
    '@src': './src',
  },
};

@alexeychikk
Copy link

If someone hits this, I fixed it like this:

  1. install graceful-fs
npm i -D graceful-fs
  1. create build.js in your projects folder:
require('graceful-fs').gracefulify(require('fs'));

const snowpack = require('snowpack');

(async () => {
  try {
    await snowpack.cli(['', '', 'build']);
    process.exit(0);
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
})();
  1. replace your build script with:
  "scripts": {
    "build": "node ./build.js"
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contributors welcome! contributors welcome! good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants