sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.48k stars 1.89k forks source link

next.360 build fails with "Maximum call stack size exceeded" for large dynamic imports #5399

Closed KeithAndreRose closed 2 years ago

KeithAndreRose commented 2 years ago

Describe the bug

Starting in next.360 my CI/CD build script failed with [vite-plugin-svelte-kit] Maximum call stack size exceeded I poked around and found it coming from recurse calls to the traverse procedure added apart of #5138

I modified the traverse function to debug

function traverse(file, add_js) {
  const chunk = manifest[file];
  console.log('traversing chunk', {chunk})

  if (imports.has(chunk.file)) return;
  if (add_js) imports.add(chunk.file);

  if (chunk.css) {
    chunk.css.forEach((file) => {
      console.log('adding stylesheet', {file})
      return stylesheets.add(file)
   });
 }

  if (chunk.imports) {
    chunk.imports.forEach((file) => {
      console.log('traversing import', {file})
        return traverse(file, add_js)
      });
  }

  if (add_dynamic_css && chunk.dynamicImports) {
    chunk.dynamicImports.forEach((file) => {
      console.log('traversing dynamic import', {file})
        return traverse(file, false)
      });
    }
}

and these where the last few lines.

Screen Shot 2022-07-06 at 5 42 57 PM

I had been dynamically importing monaco-editor in one my layouts. There are workaround importing via cdnjs however being able to dynamically import it is a lot more desired.

Script to reproduce with starter project added below.

Reproduction

npm create svelte my-app
cd my-app
cat <<EOT > src/routes/__layout.svelte
<script>
  import { onMount } from 'svelte';
  onMount(async () => {
    const monaco = await import('monaco-editor/esm/vs/editor/editor.main')
    console.log({monaco})
  })
</script>

<main>
  <slot />
</main>
EOT
npm add monaco-editor
npm run build

Logs

✓ 844 modules transformed.
.svelte-kit/output/server/manifest.json                         1.77 KiB
.svelte-kit/output/server/index.js                              73.11 KiB
.svelte-kit/output/server/entries/endpoints/todos/index.js      1.33 KiB
.svelte-kit/output/server/entries/pages/__layout.svelte.js      0.25 KiB
.svelte-kit/output/server/entries/fallbacks/error.svelte.js     0.72 KiB
.svelte-kit/output/server/entries/pages/about.svelte.js         1.51 KiB
.svelte-kit/output/server/entries/pages/index.svelte.js         9.71 KiB
.svelte-kit/output/server/entries/pages/todos/index.svelte.js   9.70 KiB
.svelte-kit/output/server/chunks/index-6892bb9c.js              3.86 KiB
.svelte-kit/output/server/chunks/hooks-ff4815b6.js              0.46 KiB
[vite-plugin-svelte-kit] Maximum call stack size exceeded
➜  my-app

System Info

System:
    OS: macOS 12.5
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
    Memory: 36.78 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.4.0 - ~/.nvm/versions/node/v18.4.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 8.12.1 - ~/.nvm/versions/node/v18.4.0/bin/npm
  Browsers:
    Brave Browser: 103.1.40.107
    Chrome: 103.0.5060.114
    Firefox Nightly: 84.0a1
    Safari: 15.6
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.54 
    @sveltejs/kit: next => 1.0.0-next.360 
    svelte: ^3.46.0 => 3.48.0 
    vite: ^2.9.13 => 2.9.13

Severity

serious, but I can work around it

Additional Information

No response

alexbjorlig commented 2 years ago

| also tried updating our project to 360 - but also get this during build:

[vite-plugin-svelte-kit] Maximum call stack size exceeded
web-app:build: error during build:
web-app:build: RangeError: Maximum call stack size exceeded

web-app:build:     at traverse (file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:216:19)
web-app:build:     at file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:36
web-app:build:     at Array.forEach (<anonymous>)
web-app:build:     at traverse (file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:18)
web-app:build:     at file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:36
web-app:build:     at Array.forEach (<anonymous>)
web-app:build:     at traverse (file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:18)
web-app:build:     at file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:36
web-app:build:     at Array.forEach (<anonymous>)
web-app:build:     at traverse (file:///Users/alexbjorlig/Documents/eddystone/node_modules/@sveltejs/kit/dist/vite.js:227:18)
tsukhu commented 2 years ago

I too got the same error after upgrading to 360 .

[vite-plugin-svelte-kit] Maximum call stack size exceeded
--
13:58:58.834 | Done in 97.07s.
13:58:58.859 | Error: No Output Directory named "public" found after the Build completed. You can configure the Output Directory in your Project Settings. Learn More: https://vercel.link/missing-public-directory

reverting to "@sveltejs/kit": "1.0.0-next.359", and then it builds fine

camargo commented 2 years ago

Getting the same error when importing monaco-editor on 360: https://github.com/NASA-AMMOS/aerie-ui/blob/develop/src/components/ui/MonacoEditor.svelte#L71

[vite-plugin-svelte-kit] Maximum call stack size exceeded
tsukhu commented 2 years ago

Looks like the common issue is monaco-editor . I too use the same and have the same issue. Hi @KeithAndreRose - you have mentioned a workaround. Would it be possible for you to share how to use the cdn import to solve this issue in the interim ?

KeithAndreRose commented 2 years ago

@tsukhu Referencing the implementation in this gist.

<script src="https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.21.2/min/vs/loader.min.js"></script>

Could be added to the <head> of src/app.html

Then the editor setup using

<script>
  import { onMount } from 'svelte';
  let container
  onMount(async () => {
    require.config({ paths: { vs: 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.21.2/min/vs' } })

    window.MonacoEnvironment = {
      getWorkerUrl: function (workerId, label) {
        return `data:text/javascript;charset=utf-8,${encodeURIComponent(`
      self.MonacoEnvironment = {
        baseUrl: 'https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.21.2/min/'
      };
      importScripts('https://cdnjs.cloudflare.com/ajax/libs/monaco-editor/0.21.2/min/vs/base/worker/workerMain.js');`)}`
      }
    }

    require(['vs/editor/editor.main'], function () {
      monaco.editor.create(container, {
        value: ['function x() {', '\tconsole.log("Hello world!");', '}'].join('\n'),
        language: 'javascript'
      })
    })
  })
</script>

<div bind:this={container}  style="width:800px;height:600px;border:1px solid grey"></div>
tsukhu commented 2 years ago

Thank you @KeithAndreRose - This worked perfectly. My vercel build time has also reduced by 20 seconds :-) after this (the transform for all the monaco libraries during the build is now gone).

camargo commented 2 years ago

I looked into this one a bit today. Essentially this is happening because of a circular dependency in Monaco, and the traverse function not adding the dynamic import from the circular dependency to the imports Set. For example here is my test component:

<script>
  import { onMount } from 'svelte';

  onMount(async () => {
    await import('monaco-editor/esm/vs/language/typescript/monaco.contribution.js');
  });
</script>

And here is my corresponding .svelte-kit/output/client/_app/manifest.json file (truncated a bit to make it more clear):

{
  "src/routes/index.svelte": {
    "file": "immutable/pages/index.svelte-93e06e99.js",
    "src": "src/routes/index.svelte",
    "isEntry": true,
    "isDynamicEntry": true,
    "imports": [
      "_preload-helper-96c8edfa.js",
      "_index-06f608d5.js"
    ],
    "dynamicImports": [
      "_monaco.contribution-0af8d1ea.js"
    ]
  },
  "_index-06f608d5.js": {
    "file": "immutable/chunks/index-06f608d5.js"
  },
  "_preload-helper-96c8edfa.js": {
    "file": "immutable/chunks/preload-helper-96c8edfa.js"
  },
  "_monaco.contribution-0af8d1ea.js": {
    "file": "immutable/chunks/monaco.contribution-0af8d1ea.js",
    "isDynamicEntry": true,
    "imports": [
      "_preload-helper-96c8edfa.js"
    ],
    "dynamicImports": [
      "node_modules/.pnpm/monaco-editor@0.33.0/node_modules/monaco-editor/esm/vs/language/typescript/tsMode.js"
    ],
    "css": [
      "immutable/assets/monaco.contribution-9deb5b7b.css"
    ],
    "assets": [
      "immutable/assets/codicon-c99115f8.ttf"
    ]
  },
  "node_modules/.pnpm/monaco-editor@0.33.0/node_modules/monaco-editor/esm/vs/language/typescript/tsMode.js": {
    "file": "immutable/chunks/tsMode-d0fba56e.js",
    "src": "node_modules/.pnpm/monaco-editor@0.33.0/node_modules/monaco-editor/esm/vs/language/typescript/tsMode.js",
    "isDynamicEntry": true,
    "imports": [
      "_monaco.contribution-0af8d1ea.js",
      "_preload-helper-96c8edfa.js"
    ]
  }
}

Here is the traverse call stack (paths simplified to make it more clear) :

traverse('src/routes/index.svelte', true);
  traverse('_monaco.contribution.js', false); // Dynamic, not added to imports Set
    traverse('tsMode.js', false); // Dynamic, not added to imports Set
      traverse('_monaco.contribution.js', false); // Non-dynamic but not added to imports Set because add_js is false
        traverse('tsMode.js', false); // Dynamic, not added to imports Set
           traverse('_monaco.contribution.js', false); // Non-dynamic but not added to imports Set because add_js is false
             traverse('tsMode.js', false); // Dynamic, not added to imports Set
                ...

Since tsMode.js is always dynamically imported from _monaco.contribution.js it never gets added to the imports Set and thus recurses until the overflow. A quick fix would be modifying traverse:

// add_js is true at first so the dynamic import is added to the imports Set for all recursive calls
chunk.dynamicImports.forEach((file) => traverse(file, add_js));

But I don't know if we want to add dynamic imports to the imports Set like this. @Rich-Harris Any ideas on this? Happy to work on a PR for this with some direction. Thanks!