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

Source map line numbers are wrong when using typescript #5182

Closed
fd0 opened this issue Jul 22, 2020 · 19 comments
Closed

Source map line numbers are wrong when using typescript #5182

fd0 opened this issue Jul 22, 2020 · 19 comments

Comments

@fd0
Copy link

fd0 commented Jul 22, 2020

Describe the bug
When using typescript within .svelte files, the generated source map references the wrong line number, it is too low.

To Reproduce

  • Checkout the template and install libraries:
npx degit sveltejs/template svelte-typescript-app
cd svelte-typescript-app
npm install
  • Modify the script in src/App.svelte like this:
<script>
    export let name;

    // test comment
    console.error("error logged from line five in App.svelte")
</script>
  • Run the app and visit it in a browser, the following message is in the JavaScript console:
error logged from line five in App.svelte     App.svelte:5
  • Now add TypeScript support according to the instructions in the blog:
node scripts/setupTypeScript.js
npm install
  • Mark the script in src/App.svelte as TypeScript:
<script lang="ts">
    export let name;

    // test comment
    console.error("error logged from line five in App.svelte")
</script>
  • Run the app, visit it again, find the following message in the browser console:
error logged from line five in App.svelte      App.svelte:3 
  • Clicking on the file name takes me to line 3 of App.svelte:
    svelte-error

I've published a reproduction repo here: https://github.com/fd0/svelte-bug-typescript-sourcemap (including package-lock.json).

Expected behavior

The error message should refer to the correct line in the source code (line 5 instead of line 3).

Information about your Svelte project:

  • Chrome 83.0.4103.116
  • Debian Buster
  • Svelte 3.24.0
  • Rollup 2.22.2

Severity

It's very annoying, but does not block me (or anybody else). Debugging takes much longer.

Additional context

Thank you very much for adding TypeScript to Svelte, I love it!

@benmccann
Copy link
Member

Thanks for the issue report and help in reproducing this. If this only happens when using TypeScript you probably will get a better response filing this issue in the corresponding subproject. Though I'm not entirely sure whether that's https://github.com/sveltejs/svelte-preprocess or https://github.com/sveltejs/language-tools (if this is an issue in svelte2tsx) - you might want to ask in the #language-tools channel on Discord

@pngwn
Copy link
Member

pngwn commented Jul 22, 2020

This is probably just because preprocessors don’t support sourcemaps. There is a PR for this somewhere.

@benmccann
Copy link
Member

sveltejs/svelte-preprocess#171 says "Added general sourceMap: boolean prop to set source maps for all preprocessors." and the reproduction repo uses svelte-preprocess v4. I'm not quite sure from the description if that is set by default or not. Perhaps you could try adding that option to the rollup config to see if it changes anything?

@fd0
Copy link
Author

fd0 commented Jul 22, 2020

Setting the sourceMap config to either true or false doesn't change anything:

            preprocess: sveltePreprocess({
                sourceMap: true,
            }),

Source maps are generated and reference the wrong line. Hm.

@Conduitry
Copy link
Member

Any sourcemaps generated by svelte-preprocess (and I don't know that it is generating any) currently won't do anything because the main compiler doesn't handle inputting sourcemaps - and so the sourcemaps in the compiled components will point to lines in the already-preprocessed versions of the components. Support for this will need to be added in Svelte (there is a PR for this somewhere that hasn't been reviewed yet), and the bundler plugins will need to be updated to pass the sourcemaps from the preprocessor to the compiler, and also the preprocessor will need to actually return sourcemaps.

@benmccann
Copy link
Member

Ah, thanks for the clarification. And here's the PR for reference: #5015

@Baxterboom
Copy link

I tried to check status on this, but gets lost in github refs. Are we close to a solution?

@samal-rasmussen
Copy link

I tried to check status on this, but gets lost in github refs. Are we close to a solution?

This PR with the bulk of the work, as far as I can see, is approved and waiting to be merged: #5584

So I believe the answer is yes we are close.

@milahu
Copy link
Contributor

milahu commented Nov 30, 2022

still an issue with import alias
this should also affect svelte-kit where import alias is used by default

repro

cd $(mktemp -d)
git clone https://github.com/milahu/svelte-web-desktop --branch debug-typescript-wrong-line-numbers
cd svelte-web-desktop
pnpm install
npm run dev

errors start at line 50

wallpaper.config.ts:1 src/configs/wallpapers/wallpaper.config.ts - line 1
wallpaper.config.ts:10 src/configs/wallpapers/wallpaper.config.ts - line 10
wallpaper.config.ts:20 src/configs/wallpapers/wallpaper.config.ts - line 20
wallpaper.config.ts:40 src/configs/wallpapers/wallpaper.config.ts - line 40
wallpaper.config.ts:43 src/configs/wallpapers/wallpaper.config.ts - line 50 -> 43
wallpaper.config.ts:71 src/configs/wallpapers/wallpaper.config.ts - line 80 -> 71
wallpaper.config.ts:440 src/configs/wallpapers/wallpaper.config.ts - line 500 -> 440

src/components/apps/WallpaperApp/Wallpaper.svelte

  import { wallpapersConfig } from '$src/configs/wallpapers/wallpaper.config';

relevant config ...

vite.config.ts

export default defineConfig({
  resolve: {
    alias: {
      '$src': path.resolve('./src')
    },
  },

tsconfig.json

{
  "compilerOptions": {
    //"baseUrl": ".",
    "paths": {
      "$src/*": ["src/*"]
    },
  },
}

i tried to patch alias imports to relative imports with
https://stackoverflow.com/questions/57188027/refactor-aliased-imports-to-relative-paths
but jscodeshift cannot parse *.svelte files

/*
https://stackoverflow.com/questions/57188027/refactor-aliased-imports-to-relative-paths

pnpm i -D jscodeshift @babel/core

# problem: jscodeshift cannot parse *.svelte files
npx jscodeshift --extensions=svelte --parser=svelte -t codemods/alias-to-relative-import.cjs src

# JS dryrun + print
npx jscodeshift -t codemods/alias-to-relative-import.cjs src -d -p

# JS
npx jscodeshift -t codemods/alias-to-relative-import.cjs src

# TS dryrun + print
npx jscodeshift --extensions=ts --parser=ts -t codemods/alias-to-relative-import.cjs src -d -p

# TS
npx jscodeshift --extensions=ts --parser=ts -t codemods/alias-to-relative-import.cjs src

*/

const path = require("path");

function replacePathAlias(currentFilePath, importPath, pathMap) {
  // if windows env, convert backslashes to "/" first
  currentFilePath = path.posix.join(...currentFilePath.split(path.sep));

  const regex = createRegex(pathMap);
  return importPath.replace(regex, replacer);

  function replacer(_, alias, rest) {
const mappedImportPath = pathMap[alias] + rest;

// use path.posix to also create foward slashes on windows environment
let mappedImportPathRelative = path.posix.relative(
  path.dirname(currentFilePath),
  mappedImportPath
);
// append "./" to make it a relative import path
if (!mappedImportPathRelative.startsWith("../")) {
  mappedImportPathRelative = `./${mappedImportPathRelative}`;
}

logReplace(currentFilePath, mappedImportPathRelative);

return mappedImportPathRelative;
  }
}

function createRegex(pathMap) {
  const mapKeysStr = Object.keys(pathMap).reduce((acc, cur) => `${acc}|${cur}`);
  const regexStr = `^(${mapKeysStr})(.*)$`;
  return new RegExp(regexStr, "g");
}

const log = true;
function logReplace(currentFilePath, mappedImportPathRelative) {
  if (log)
console.log(
  "current processed file:",
  currentFilePath,
  "; Mapped import path relative to current file:",
  mappedImportPathRelative
);
}

/**
 * Corresponds to tsconfig.json paths or webpack aliases
 * E.g. "@/app/store/AppStore" -> "./src/app/store/AppStore"
 */
const pathMapping = {
  //"@": "./src",
  //"foo": "bar",
  "$src": "./src",
};

module.exports = function transform(file, api) {
  const j = api.jscodeshift;
  const root = j(file.source);

  root.find(j.ImportDeclaration).forEach(replaceNodepathAliases);
  root.find(j.ExportAllDeclaration).forEach(replaceNodepathAliases);

  /**
   * Filter out normal module exports, like export function foo(){ ...}
   * Include export {a} from "mymodule" etc.
   */
  root
.find(j.ExportNamedDeclaration, (node) => node.source !== null)
.forEach(replaceNodepathAliases);

  return root.toSource();

  function replaceNodepathAliases(impExpDeclNodePath) {
impExpDeclNodePath.value.source.value = replacePathAlias(
  file.path,
  impExpDeclNodePath.value.source.value,
  pathMapping
);
  }
};

@umaranis
Copy link

umaranis commented Dec 3, 2022

@milahu I am facing the same issue

@benmccann
Copy link
Member

@milahu this sounds like a bug in either Vite (e.g. this plugin) or @rollup/plugin-alias. It'd be great if you were able to narrow down which project is causing it! Thanks as always for your help improving source map support!

@umaranis
Copy link

umaranis commented Dec 4, 2022

@milahu @benmccann not sure it's relevant to this issue but I get the following warning from vite-plugin-svelte.
 WARN  deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead

@benmccann
Copy link
Member

@umaranis can you file an issue in the sveltekit repo with instructions to reproduce it? I've never seen that warning. Not sure if I use a different package manager than you or just tuned it out or what

@milahu

This comment was marked as spam.

@milahu
Copy link
Contributor

milahu commented Dec 5, 2022

fixed in the vite main branch

this affects only development mode, vite build works

@benmccann
Copy link
Member

@milahu I reverted that PR two days ago. Just making sure, did you clone the repo since then? Or alternatively, does it work with 4.0.0-beta.0?

@benmccann
Copy link
Member

Here's details about the sourcemap issue in Vite that was attempted to be fixed and then reverted: vitejs/vite#7767 (comment)

@milahu
Copy link
Contributor

milahu commented Dec 5, 2022

I reverted that PR two days ago.

sorry, ninja edit

vitejs/vite@4c85c0a fails
vitejs/vite@14980a1 works

which makes sense, because my code has

const optimizedWallpapers = import.meta.glob('../../assets/wallpapers/*.{webp,jpg}', {
  eager: true,
  query: { width: 2000, quality: 95, format: 'webp' },
}) as Record<string, NodeModule>;

@benmccann
Copy link
Member

Awesome! Glad it's fixed!!

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

No branches or pull requests

9 participants