Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

dev flag used by Rollup/webpack configs seems reversed #590

Closed
jarrodldavis opened this issue Mar 3, 2019 · 5 comments · Fixed by #771
Closed

dev flag used by Rollup/webpack configs seems reversed #590

jarrodldavis opened this issue Mar 3, 2019 · 5 comments · Fixed by #771

Comments

@jarrodldavis
Copy link
Contributor

It appears that source maps are not enabled for sapper dev but are enabled for sapper build. I'm using the sapper/svelte v3 rollup template, and the issue appears to be at the call sites of create_compilers in the CLI command implementations.

Here's some relevant code from some GitHub spelunking:

sapper dev:

import { create_manifest_data, create_app, create_compilers, create_serviceworker_manifest } from '../core';

const compilers: Compilers = await create_compilers(this.bundler, cwd, src, dest, false);

sapper build:

import { create_compilers, create_app, create_manifest_data, create_serviceworker_manifest } from '../core';

const { client, server, serviceworker } = await create_compilers(bundler, cwd, src, dest, true);

create_compilers:

import { set_dev, set_src, set_dest } from '../../config/env';

export default async function create_compilers(
bundler: 'rollup' | 'webpack',
cwd: string,
src: string,
dest: string,
dev: boolean
): Promise<Compilers> {
set_dev(dev);
set_src(src);
set_dest(dest);

env.ts:

export let dev: boolean;
export let src: string;
export let dest: string;
export const set_dev = (_: boolean) => dev = _;
export const set_src = (_: string) => src = _;
export const set_dest = (_: string) => dest = _;

Rollup Config (sapper/config/rollup.js):

import { dev, src, dest } from './env';

sourcemap: dev

sourcemap: dev

Should I go ahead and submit a pull request to reverse the values passed into create_compilers, or is there something I'm not understanding?

@thgh
Copy link
Contributor

thgh commented Mar 3, 2019

JS client sourcemaps seem to be useful:

Dev js without sourcemap (current)

chunk.d6bddd73.js:71 Uncaught (in promise) Error: client js error
    at onMount (chunk.d6bddd73.js:71)
    at run (chunk.666eb2d2.js:15)
    at Array.map (<anonymous>)
    at add_render_callback (chunk.666eb2d2.js:260)
    at flush (chunk.666eb2d2.js:191)
    at init (chunk.666eb2d2.js:354)
    at new App (client.d3c9d90e.js:851)
    at render (client.d3c9d90e.js:1182)

Dev js with sourcemap

about.svelte:13 Uncaught (in promise) Error: client js error
    at onMount (about.svelte:13)
    at run (internal.mjs:21)
    at Array.map (<anonymous>)
    at add_render_callback (internal.mjs:1290)
    at flush (internal.mjs:936)
    at init (internal.mjs:1384)
    at new App (App.svelte:15)
    at render (app.mjs:195)

What tool can read server.js.map?

CSS sourcemaps are generated in both build & dev. And never working.

@jarrodldavis
Copy link
Contributor Author

I actually found this issue because I was wondering why the server.js sourcemap wasn't present in dev. VS Code's node.js debugger can use source maps so that breakpoints work e.g. in server routes.

@Seb35
Copy link
Contributor

Seb35 commented Apr 11, 2019

I also remarked this. I have no opinion about build mode (it could be argumented that it could be useful even in prod) but it should be enabled in dev mode. See also #537 about CSS sourcemaps.

I push two distinct PRs: one in dev, one in build.

Seb35 added a commit to Seb35/sapper that referenced this issue Apr 11, 2019
Seb35 added a commit to Seb35/sapper that referenced this issue Apr 11, 2019
@Seb35
Copy link
Contributor

Seb35 commented Apr 11, 2019

Thinking about this, I can see the initial reasoning:

  • in dev mode, JS source files are not minified, hence sourcemaps are useless,
  • in build mode, JS source files are minified, hence sourcemaps are useful.

Possibly this issue is wontfix, but in this case the variables names should be changed to improve readability and/or comments could be added.

@jarrodldavis
Copy link
Contributor Author

As I mentioned earlier, source maps are still useful in development when debugging server routes:

I actually found this issue because I was wondering why the server.js sourcemap wasn't present in dev. VS Code's node.js debugger can use source maps so that breakpoints work e.g. in server routes.

Additionally, source maps also map (as their name imply) back to the original source file; since both server- and client-side code is transformed (by Svelte) and bundled (by Rollup) even in development, it is useful to see the original per-component/per-route source, especially when setting breakpoints.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants