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

feat(source-maps): bundling improvements #751

Merged
merged 1 commit into from Oct 12, 2017
Merged

feat(source-maps): bundling improvements #751

merged 1 commit into from Oct 12, 2017

Conversation

dima-v
Copy link
Contributor

@dima-v dima-v commented Sep 12, 2017

multiple fixes to source maps:

  • fixes offset problems caused by:
    • prepended scripts;
    • duplicate dependency main module;
  • remove sourece map file comments;
  • add EOL separator for the anonymous module wrapper;

closes #659, related to #624

@CLAassistant
Copy link

CLAassistant commented Sep 12, 2017

CLA assistant check
All committers have signed the CLA.

@simonfox
Copy link
Contributor

@dima-v have been discussing this with @JeroenVinke over on Gitter...couple of queries

  1. what was the reason for modifying the determination of the source root
  2. why the columnOffset calculation is moved to apply to all files

Below is @JeroenVinke comment with details...

I found one issue though, when I tried to use aurelia-configuration with these changes.
It looked like this:

image

The index.ts, aurelia-configuration.ts and deep-extend.ts should not be in the src directory, so I dug into the code.
The variables here have the following values:
aurelia-configuration
/node_modules/aurelia-configuration/dist/amd
/node_modules/aurelia-configuration
Thus the source root is set to /node_modules/aurelia-configuration. If we look at the sourcemap, this one then the sources are defined as ../../src/aurelia-configuration.ts, so when the sourceRoot is node_modules/aurelia-configuration then Chrome will think that those sources belong in the src folder.
So I changed the code to:

if (sourceMap !== null) {
  let sourceRoot = parsedPath.dir.substring(process.cwd().length);
  sourceMap.sourceRoot = sourceRoot.replace(/\\/g, '\/');
}

and that fixed it. Then it works perfectly. I'd like to know the reason for changing this code though, and how I can reproduce the issue that you had so I can confirm the fix.
Much thanks!
If you could explain the issues that you've had for which you changed those two bits of code (the offsets) and the sourceRoot, then that would help me a lot

multiple fixes to source maps:
- fixes offset problems caused by:
	- prepended scripts;
	- duplicate dependency main module;
- remove sourece map file comments;
- add EOL separator for the anonymous module wrapper;

closes #659, related to #624
@dima-v
Copy link
Contributor Author

dima-v commented Oct 12, 2017

@JeroenVinke thank you for your suggestion on how to fix the source root issue.
It works for us and I updated the pull request with related changes.

Regarding to the column offset calculation ...

with the following configuration for a bundle file:

  • prepend files or/and dependency package with resources (such as html, css or js files)

we found that resulting map file for the bundle will have incorrect mapping and will cause unpredictable behavior when debugging. The offset needs to consider all files in the bundle, not just those which have a sourcemap associated.

we used that tool http://sokra.github.io/source-map-visualization/#custom to find the issue.

Please let me know if you need a working example.

@JeroenVinke
Copy link
Collaborator

Thanks for the quick response @dima-v. Nice work

@JeroenVinke JeroenVinke merged commit c845fd0 into aurelia:master Oct 12, 2017
@dima-v dima-v deleted the feature/sourcemap branch October 12, 2017 20:46
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

Successfully merging this pull request may close these issues.

Getting "Unexpected token <" error since using SystemJS
4 participants