sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.6k stars 4.12k forks source link

Error: offset is longer than source length w/ Svelte v3.38.3 #6440

Open akaufmann opened 3 years ago

akaufmann commented 3 years ago

Since the update to the latest Svelte version (3.38.3) an error is thrown from Vite when a request comes in.

Logs

$ svelte-kit dev --port 8000

  SvelteKit v1.0.0-next.116

  local:   http://localhost:8000
  network: not exposed

  Use --host to expose server to other devices on this network

offset is longer than source length!
Error: offset is longer than source length!
    at numberToPos (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:4234:15)
    at formatError (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44611:24)
    at TransformContext.error (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44591:19)
    at Object.transform (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44799:25)
    at async transformRequest (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:60267:29)
    at async instantiateModule (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:69930:10)

I traced it to an error in one of my style blocks not displaying because the offset/source length is incorrect:

ERR:: CompileError [ValidationError]: :global(...) must contain a single selector
    at error (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:16752:19)
    at Component.error (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:29036:9)
    at Selector$1.validate (file:///<path/to/the/project>t/node_modules/svelte/compiler.mjs:27872:35)
    at file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:28369:22
    at Array.forEach (<anonymous>)
    at Rule$1.validate (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:28368:24)
    at file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:28650:19
    at Array.forEach (<anonymous>)
    at Stylesheet.validate (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:28649:23)
    at new Component (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:28828:25) {
  code: 'css-invalid-global-selector',
  start: { line: 89, column: 10, character: 2845 },
  end: { line: 89, column: 58, character: 2893 },
  pos: 2845,
  ...
  frame: '87:   }\n' +
    '88: \n' +
    '89:   .slider :global(.noUi-handle:after, .noUi-handle:before) {\n' +
    '              ^\n' +
    '90:     left: 10px !important;\n' +
    '91:     top: 4px !important;',
  ...

The offset is 2845 (offset/pos) but the source length is 2417 in that case. https://github.com/vitejs/vite/blob/460d1cda317e4c4d03434f2b3d8de9152620005b/packages/vite/src/node/utils.ts#L252

export function numberToPos(
  source: string,
  offset: number | { line: number; column: number }
): { line: number; column: number } {
  if (typeof offset !== 'number') return offset
  if (offset > source.length) {
    throw new Error('offset is longer than source length!')
  }
  const lines = source.split(splitRE)

Information about your SvelteKit Installation:

System:
    OS: macOS 10.15.4
    CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
    Memory: 367.08 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 16.0.0 - ~/.nvm/versions/node/v16.0.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.10.0 - ~/.nvm/versions/node/v16.0.0/bin/npm
  Browsers:
    Brave Browser: 76.0.68.132
    Chrome: 91.0.4472.114
    Chrome Canary: 93.0.4551.0
    Edge: 91.0.864.54
    Firefox: 72.0.2
    Firefox Developer Edition: 89.0
    Safari: 13.1
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0-next.27 => 1.0.0-next.27 
    @sveltejs/adapter-static: ^1.0.0-next.13 => 1.0.0-next.13 
    @sveltejs/kit: ^1.0.0-next.116 => 1.0.0-next.116 
    svelte: 3.38.3 => 3.38.3 

Let me know if this info is sufficient and reproducible with any SvelteKit app with the latest Svelte version + multiple selectors in a single :global like :global(.foo, .bar) or a repro is needed.

PS: Not sure if the bug report is correct here or should be created in the svelte or vite-plugin-svelte repo.

benmccann commented 3 years ago

Let me know if this info is sufficient and reproducible with ... multiple selectors in a single :global

That sounds like you're trying to do something unsupported by Svelte because you're getting an error message that says it's not allowed:

ERR:: CompileError [ValidationError]: :global(...) must contain a single selector

akaufmann commented 3 years ago

Yes, that is the root cause, but the calculation of the offset or source length is not correct from Svelte. This is the reason why you don't see the actual problem and therefore rather difficult to detect unless you dig into Vite's source code. So it's more of a general problem with source transpilation and calculating the correct positions and source length.

EDIT: Btw, it would be nice if the language service would detect this, as it was not a problem with Svelte < v3.38.3 and was introduced with this version, but this is an additional feature request :)

benmccann commented 3 years ago

I'm not quite sure where offset and source length are coming from, but I don't think it's SvelteKit? Maybe this is a bug in Svelte, Vite, or vite-plugin-svelte?

akaufmann commented 3 years ago

Ok, I think I found the problem: Svelte calculates the offset/pos based on the transpiled source:

// props.source which is the component source https://github.com/sveltejs/svelte/blob/228832c9a3a6fdb8c11cdbb942444734686a04f6/src/compiler/utils/error.ts#L28
stylesheet: Stylesheet {
 ...
  source: '<script lang="ts">var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {\n' +
    '    function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }\n' +
    '    return new (P || (P = Promise))(function (resolve, reject) {\n' +
    '        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }\n' +
    '        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }\n' +
    '        function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); }\n' +
    '        step((generator = generator.apply(thisArg, _arguments || [])).next());\n' +
    '    });\n' +
    '};\n' +
    'import { onMount } from "svelte";\n' +
    ...
      '\n' +
      '<style>\n' +
      ...
      '  .slider :global(.noUi-handle:after, .noUi-handle:before) {\n' +
      '    left: 10px !important;\n' +
      '    top: 4px !important;\n' +
      '  }\n' +
      ...
      ' *{}</style>\n',
      ...
    },
    ...

...and adds it to the error object (error.pos) that Vite uses. But Vite uses the non-transpiled file for the source length:

// source object https://github.com/vitejs/vite/blob/460d1cda317e4c4d03434f2b3d8de9152620005b/packages/vite/src/node/utils.ts#L257
<script lang="ts">
  import { onMount } from "svelte"
  ...
</script>
...  
<style>
   ...
  .slider :global(.noUi-handle:after, .noUi-handle:before) {
    left: 10px !important;
    top: 4px !important;
  }
  ...
</style>

...which, of course, does not match. This is a Svelte and not a SvelteKit issue.

dummdidumm commented 3 years ago

Did you enable source maps for TS? I guess you use svelte-preprocess, there's a corresponding setting: https://github.com/sveltejs/svelte-preprocess/blob/main/docs/preprocessing.md#auto-preprocessing

Maybe we should enable that by default in the starter template.

akaufmann commented 3 years ago

@dummdidumm thanks! The problem was that I set postcss to true (I had thought that I needed it for Tailwind)

    sveltePreprocess({
      defaults: {
        script: 'typescript',
        style: 'postcss',
      },
      postcss: true,
    }),

When I remove it the language service shows the error in VSC.

dummdidumm commented 3 years ago

Yes, it's a known problem that PostCSS does not produce source maps and therefore the chain of source maps breaks. Not sure if there are possibilities for svelte-preprocess to catch this and provide sane fallbacks. Either way, I'm going to close this since it's neither directly related to SvelteKit nor Svelte.

akaufmann commented 3 years ago

@dummdidumm sorry, it's still a svelte problem that the offset is still passed to Vite incorrectly, although the language service shows the error in VSC. Yes, the error only occurs (for me) when I have used the :global incorrectly, but this could be a problem in other situations as well.

dummdidumm commented 3 years ago

Ok I'll reopen until the culprit is found, but I still think this is a source map issue of third parties involved. Svelte can't magically know what the offsets are if there's no proper source map passed to it.

Also please try setting source maps to true explicitely:

    sveltePreprocess({
      defaults: {
        script: 'typescript',
        style: 'postcss',
      },
      sourceMap: true // <---------
    }),
akaufmann commented 3 years ago

I did that but it does not prevent the error. I added the error.pos (offset) from Svelte (2845) and the source length from Vite (2515) to Vite's error message:

offset is longer than source length! 2845 2515
Error: offset is longer than source length! 2845 2515
    at numberToPos (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:4234:15)
    at formatError (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44612:24)
    at TransformContext.error (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44591:19)
    at Object.transform (/<path/to/the/project>/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js:44802:25)

The offset is set from the transpiled file from Svelte and added to the error object (error.pos) which could be okay but Vite uses the non-transpiled file and checks if the length of it is greater than the offset (error.pos), which might be the case if the error occurs further up but then the offset is still not correct.

I moved the wrong :global up as a test, so the offset (error.pos) is definitely lower than the source length and then I see the correct error:

:global(...) must contain a single selector
ValidationError: :global(...) must contain a single selector
    at error (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:16752:19)
    at Component.error (file:///<path/to/the/project>/node_modules/svelte/compiler.mjs:29038:9)

tl;dr: If an error occurred at the end of the Svelte file, the offset is higher than the source length (not transpiled) because the error position from Svelte is taken from the transpiled file. If an error occurs at a position where the offset is still within the range of the source length, then everything is fine, but if you were to compare the error position (offset) from Svelte and the error position from the Vite's file source, it would not match.

EDIT: Either Vite should use the transpiled Svelte file as source (not sure where this is coming from) or the error.pos from Svelte should be set from the non-transpiled file.

benmccann commented 3 years ago

I would guess the latter. You probably want to know the location in the original source since that's what the user sees. @milahu did quite a bit of work on getting our sourcemaps working earlier and might be interested in this

milahu commented 3 years ago

@akaufmann can you help to reproduce the error? something like ...

reproduce.sh ```bash #!/usr/bin/env bash # reproduce bug in svelte-preprocess # Error: offset is longer than source length! set -o xtrace # print cmds expect << EOF # generated by autoexpect set timeout -1 spawn pnpx create-svelte@next my-app match_max 100000 expect -re "Install the following package: create-svelte@next?" # yes send -- "\r" expect -re "Which Svelte app template?" # default (svelte kit) send -- "\r" expect -re "Use TypeScript?" # right -> yes send -- "\033\[C" send -- "\r" expect -re "Add ESLint for code linting?" # no send -- "\r" expect -re "Add Prettier for code formatting?" # no send -- "\r" expect eof EOF cd my-app cat >src/routes/index.svelte << 'EOF'
Hello world!
EOF pnpm install # only in dev mode: # Error: offset is longer than source length! npm run dev -- --open ```

edit: async code + dev mode

akaufmann commented 3 years ago

@milahu you get the error with a Svelte file like this:

<script lang="ts">
  import { onMount } from 'svelte';

  onMount(async () => {
     const delay = (ms: number) => new Promise((res) => setTimeout(res, ms));
     await delay(3000);
  });
</script>

<div>Hello world!</div>

<style>
  .foo :global(.bar, .baz) {
    left: 10px;
    top: 4px;
  }
</style>

Just create a SvelteKit demo app with npm init svelte@next offset-bug, add TS so that the async/await in the file is transpiled so we can trigger the offset bug more easily and add the file e.g. as /src/routes/index.svelte. Start the dev server and open the browser. The first request should trigger it.

milahu commented 3 years ago

thanks, i fixed my reproduce.sh script

currently class Component has no original source which we need in Component.error and Component.warn

proposed solution: add the original source to Component.compile_options.sourcemap.sourcesContent if the original source is missing, return location in transformed source

dummdidumm commented 3 years ago

Sounds related to #6089 , is that correct? Edit: No it's not directly related. What you mean is that when an error or warning is thrown, the positions are those of the final source that Svelte sees, positions are not mapped back, the map passed in is not used here. We have to map these positions oursleves in language-tools because of that: https://github.com/sveltejs/language-tools/blob/master/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts . If this is implemented, beware of invalid mapping ranges, we had to add some extra logic for this: https://github.com/sveltejs/language-tools/pull/1035

milahu commented 3 years ago

when an error or warning is thrown, the positions are those of the final source that Svelte sees, positions are not mapped back, the map passed in is not used here.

yes

Component.error result is correct: error.props.start and error.props.end are relative to the preprocessed source error.props.source (result of svelte-preprocess) also error.start.line etc. are correct

but somewhere between server.pluginContainer.transform and vite utils numberToPos error.props.start and error.props.end are used with the original source (input for svelte-preprocess)

one solution is to make Component.error return locations relative to the original source, but then error.props.source also should be the original source

or we fix the problem later in the toolchain

dummdidumm commented 3 years ago

Good question, what would make semantically more sense? I'm split here .. more opinions would be good.

milahu commented 3 years ago

what would make semantically more sense?

ideally both: we need the generated source to understand the error we need the original source to find the source

one solution is to make Component.error return locations relative to the original source

started here

problem: low-resolution sourcemaps this means: of the generated source, only some lines are mapped

<style>/* mapped: 19:6 -> original 13:6 */
/* not mapped */
  :global(.a, .b) { color: red; } /* location of error - not mapped */
/* not mapped */
</style><!-- mapped -->

-> backtracing :global(.a, .b) to its original source is non-trivial

edit: at least in this case, i can backtrace the unmapped segment by calculating the char_offset of the first previous mapped segment see svelte/src/compiler/utils/error.ts

milahu commented 3 years ago

related: https://github.com/vitejs/vite/pull/4782