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

Customize sourcemap sources paths #2168

Closed
pgn-vole opened this issue May 2, 2018 · 16 comments
Closed

Customize sourcemap sources paths #2168

pgn-vole opened this issue May 2, 2018 · 16 comments

Comments

@pgn-vole
Copy link

pgn-vole commented May 2, 2018

It doesn't seem possible at the moment to customize the path the sources within the sourcemap.

I would like the sources within the sourcemap to be relative to the output file.
Instead of :

 "file": "myLib.min.js",
  "sources": [
    "../../node_modules/fbjs/lib/emptyFunction.js",
    "../../src/Header.js",
  ],

I would like :

 "file": "myLib.min.js",
  "sources": [
    "./node_modules/fbjs/lib/emptyFunction.js",
    "./src/Header.js",
  ],

This makes the sourcemaps easier to be read by consumers of the library as the paths of the sources will always be relative to location of the bundle.
This kind of configuration is possible with browserify and Webpack (although a bit buggy with Webpack). So I guess it should be achieveable by the amazing Rollup.

@guybedford
Copy link
Contributor

Certainly we can look at a configuration option for this.

How does it look in Webpack in this scenario?

@pgn-vole
Copy link
Author

pgn-vole commented May 8, 2018

With browserify you can use a plugin https://github.com/thlorenz/mold-source-map. It is really easy to configure.
With Webpack you can configure this option : https://webpack.js.org/configuration/output/#output-devtoolmodulefilenametemplate . It is pretty buggy though.

@teohhanhui
Copy link

teohhanhui commented Jun 6, 2018

With gulp-sourcemaps I could do something like:

// src/AppBundle/gulpfile.babel.js
import concat from 'gulp-concat';
import gulp from 'gulp';
import gulpif from 'gulp-if';
import sourcemaps from 'gulp-sourcemaps';
import uglify from 'uglify';
import upath from 'upath';

const env = process.env.GULP_ENV;
const adminRootPath = '../../web/assets/app/admin';

const paths = {
  admin: {
    js: [
      'Resources/private/admin/js/**',
    ],
  },
};

const sourcePathMap = [
  {
    sourceDir: 'Resources/private/',
    destPath: '/AppBundle',
  },
];

const mapSourcePath = function mapSourcePath(sourcePath /* , file */) {
  const match = sourcePathMap.find(({ sourceDir }) => (
    sourcePath.substring(0, sourceDir.length) === sourceDir
  ));

  if (!match) {
    return sourcePath;
  }

  const { sourceDir, destPath } = match;

  return upath.joinSafe(destPath, sourcePath.substring(sourceDir.length));
};

export const buildAdminJs = function buildAdminJs() {
  return gulp.src(paths.admin.js, { base: './' })
    .pipe(gulpif(env !== 'prod', sourcemaps.init()))
    .pipe(concat('app.js'))
    .pipe(gulpif(env === 'prod', uglify()))
    .pipe(gulpif(env !== 'prod', sourcemaps.mapSources(mapSourcePath)))
    .pipe(gulpif(env !== 'prod', sourcemaps.write('./')))
    .pipe(gulp.dest(upath.joinSafe(adminRootPath, 'js')));
};
buildAdminJs.description = 'Build admin js assets.';

@guybedford
Copy link
Contributor

@teohhanhui would you like to propose a solution here you think might be appropriate for Rollup?

@teohhanhui
Copy link

Offtopic, but I've just realized you're the creator of SystemJS, jspm and Rollup.

That's incredible!

p/s: I used jspm a couple years ago.

@guybedford
Copy link
Contributor

@teohhanhui I thought I recognized your name there :) I'm not the creator of Rollup though - that is @Rich-Harris, although I am a core contributor currently.

@pgn-vole
Copy link
Author

pgn-vole commented Jun 7, 2018

@guybedford, I would think that an API allowing to configure the root of the sourcemap would be well fitted.
For instance given such configuration:

export default {
  input: 'src/main.js',
  output: {
    file: 'myLib.min.js',
    format: 'cjs',
    sourcemap: true,
    sourcemapRoot: 'foo'
  }
}

I would expect the sourcemaps to declare files relative to foo.

 "file": "myLib.min.js",
  "sources": [
    "foo/node_modules/fbjs/lib/emptyFunction.js",
    "foo/src/Header.js",
  ],

By default the sourcemap root would be path.resolve(<outputFile>,<file>) to follow the current behavior.

@teohhanhui
Copy link

I think a sourcemapRoot is great, but we also need a mapping function (perhaps sourcemapPathTransform) so that we can transform the source paths as needed.

@pgn-vole
Copy link
Author

pgn-vole commented Jun 7, 2018

@teohhanhui good point. I guess we don't need sourcemapRoot then. Something more generic like sourcemapPathTransform could do the job.

export default {
  input: 'src/main.js',
  output: {
    file: 'dist/myLib.min.js',
    format: 'cjs',
    sourcemap: true,
    sourcemapPathTranform: (file) => {
      return `foo/${file}`
    }
  }
}

@teohhanhui
Copy link

I don't think that works. Because we need to have a fixed base / root path to be able to perform the transform. See my (incomplete) code example above.

@teohhanhui
Copy link

Uhh... I see you had something else in mind with the sourcemapRoot. We need a sourcemapRootDir / sourcemapBaseDir.

@teohhanhui
Copy link

I've updated the above example to show how the whole thing works for us now with gulp-sourcemaps.

@teohhanhui
Copy link

teohhanhui commented Jun 7, 2018

So, if I were to imagine the API for rollup, it might work something like this:

// src/AppBundle/gulpfile.babel.js
import { rollup } from 'rollup';
import babel from 'rollup-plugin-babel';
import commonjs from 'rollup-plugin-commonjs';
import gulp from 'gulp';
import resolve from 'rollup-plugin-node-resolve';
import upath from 'upath';

const env = process.env.GULP_ENV;
const adminRootPath = '../../web/assets/app/admin';
const nodeModulesPath = '../../node_modules';

const paths = {
  admin: {
    js: [
      'Resources/private/admin/js/**',
    ],
  },
};

const sourcePathMap = [
  {
    sourceDir: 'Resources/private/',
    destPath: '/AppBundle',
  },
];

const mapSourcePath = function mapSourcePath(sourcePath) {
  const match = sourcePathMap.find(({ sourceDir }) => (
    sourcePath.substring(0, sourceDir.length) === sourceDir
  ));

  if (!match) {
    return sourcePath;
  }

  const { sourceDir, destPath } = match;

  return upath.joinSafe(destPath, sourcePath.substring(sourceDir.length));
};

export const buildAdminJs = async function buildAdminJs() {
  const bundle = await rollup({
    input: 'Resources/private/admin/js/app.js',
    external: [
      'jquery',
    ],
    plugins: [
      resolve({
        jail: upath.resolve(nodeModulesPath),
      }),
      commonjs({
        include: `${nodeModulesPath}/**`,
      }),
      babel({
        exclude: `${nodeModulesPath}/**`,
        runtimeHelpers: true,
      }),
    ],
  });

  await bundle.write({
    file: upath.joinSafe(adminRootPath, 'js/app.js'),
    format: 'iife',
    name: '_sylius.app.admin',
    globals: {
      jquery: 'jQuery',
    },
    sourcemap: true,
    sourcemapBaseDir: '.',
    sourcemapPathTransform: mapSourcePath,
  });
};
buildAdminJs.description = 'Build admin js assets.';

@sebinsua
Copy link
Contributor

sebinsua commented Aug 28, 2018

I've also wished for this feature.

If you are using rollup to produce packages with source maps inside a monorepo this can be painful, since they are created with "sources": ["../src/someFile.js"] type source mappings.

If you eventually bundle these into a webpack build this will cause everything to belong to a single src directory.

@sebinsua
Copy link
Contributor

sebinsua commented Aug 28, 2018

@guybedford @Rich-Harris Anything wrong with applying a function here to further transform the source path?

It seemed to work if I did the above and then passed an extra config option sourceMapPathTransform (function) here.

Is this solution okay? I could create a PR if it is.

@teohhanhui
Copy link

@sebinsua As long as all paths are first normalized to the same base directory... Or is that already the case?

sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 28, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
sebinsua added a commit to sebinsua/rollup that referenced this issue Aug 29, 2018
ShannonLCapper pushed a commit to textioHQ/tsdx that referenced this issue Jul 31, 2019
… name

This fixes an issue where all sourcemap paths are relative to the dist directory (eg '../src/foo/bar.ts'), which causes them to all point to the same (wrong) directory when bundled by a consumer's webpack rollup/rollup#2168 (comment)
ShannonLCapper pushed a commit to textioHQ/tsdx that referenced this issue Jul 31, 2019
… name

This fixes an issue where all sourcemap paths are relative to the dist directory (eg '../src/foo/bar.ts'), which causes them to all point to the same (wrong) directory when bundled by a consumer's webpack rollup/rollup#2168 (comment)
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

4 participants