Fix colocated source maps

[wagenet]

Resolves #558.

[dagroe]

Thanks for you MR! I'd really like to see this issue fixed for easier debugging.

I tried it on our app, but unfortunately don't get it to work properly.

Without this fix we get sourcemaps for our application-name.js that point to our components' .ts files, although with the offset in line numbers.

With this fix we get sourcemaps for our application-name.js that point to the transpiled .js files. The mapping to .ts files is missing.

When looking in to it, it seems your changes correctly apply the template prefix and add the sourcemap to our .ts files. But then those sourcemaps seem to be incompatible with the babel transpilation.

[wagenet]

@dagroe Yeah, this is the problem I have. I expected this change to fix stuff, but it doesn't seem to have. I have to dig in more. These source maps should be ingestible by other tools, so I'm not sure what's going on there. Maybe we have to change a downstream setting.

[dagroe]

@wagenet Glad I am not the only one with this problem 😉

I did a bit more digging.

We do not get sourcemaps since the generated sourcemaps fail validation. When sourcemaps are built by broccoli-concat it uses fast-sourcemap-concat which in turn runs validation via sourcemap-validator. Here validation simply fails because our sourcemaps are missing the correct column positions.

Luckily we can fix that by passing hires: true to jsContentsMagic.generateMap.

Second issue is that the sourcemaps do not have the correct paths to sources. E.g. a component in components/list/item.ts will only have item.ts in sources. What works for me was the following:

Store the correct extension:

let ext;
if (this.inputHasFile(basePath + '.js')) {
  backingClassPath += '.js';
  ext = '.js';
  hasBackingClass = true;
} else if (this.inputHasFile(basePath + '.ts')) {
  backingClassPath += '.ts';
  ext = '.ts';
  hasBackingClass = true;
} else if (this.inputHasFile(basePath + '.coffee')) {
  backingClassPath += '.coffee';
  ext = '.coffee';
  hasBackingClass = true;
} else {
  backingClassPath += '.js';
  ext = '.js';
  hasBackingClass = false;
}

then use it so set the correct file and source:

let file = filePathParts.name + ext;
let jsContentsMap = jsContentsMagic.generateMap({
  source: backingClassPath,
  file,
  includeContent: true,
  hires: true
});

This seem to generate correct sourcemaps.

[wagenet]

@dagroe Great! I'm so glad you were able to look into this. Would you prefer to make your own PR or should I make the fixes in my PR when I'm able?

[dagroe]

@wagenet Thanks for merging my changes :)

Maybe we can get someone else to test this as well. I know it's been some time but maybe @beddueimpossibile or @bartocc are still interested?

I'll do some more testing. So far source maps look fine when debugging in chrome or firefox but I haven't been able to make it work correctly in pycharm yet.

[bartocc]

Thx for flagging me! I am still very interested and I would love to see this fixed! Gonna try this PR right now!

[bartocc]

I've done some testing on our app and with VSCode.

I've changed package.json with

- "ember-cli-htmlbars": "^6.1.0",
+ "ember-cli-htmlbars": "wagenet/ember-cli-htmlbars#fix-colocated-template-sourcemaps",

setup VSCode launch setting with:

"sourceMapPathOverrides": {
  "my-app/*": "${workspaceFolder}/app/*"
}

Unfortunately, I do not see any improvements.
Here are my results:

classic structure + JS component class: breakpoint binds

app/components/foo-bar.js
app/templates/components/foo-bar.hbs

classic structure + TS component class: breakpoint binds

app/components/foo-bar.ts
app/templates/components/foo-bar.hbs

flat structure (ie co-located template) + JS component class: breakpoint DOES NOT BIND

app/components/foo-bar.js
app/components/foo-bar.hbs

flat structure (ie co-located template) + TS component class: breakpoint DOES NOT BIND

app/components/foo-bar.ts
app/components/foo-bar.hbs

[dagroe]

@bartocc I tested it using a new dummy project, which I put here: https://github.com/dagroe/test-ember-sourcemaps

The breakpoint in components/test/foo-bar.ts was hit using vscode with this launch.json:

{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "type": "chrome",
            "request": "launch",
            "name": "Launch Chrome against localhost",
            "url": "http://localhost:4300",
            "webRoot": "${workspaceFolder}",
            "sourceMapPathOverrides": {
                "html-bars-test/*": "${workspaceRoot}/app/*"
            },
        }
    ]
}

Still trying to figure it out for pycharm, but the sourceMapPathOverrides seems to be the critical part in vscode.

[bartocc]

Thx for setting up this demo project @dagroe. I need to check our own project to see why the breakpoints do not bind for co-located components…

But if I am correct, everything works fine when using wagenet/ember-cli-htmlbars#fix-colocated-template-sourcemaps, doesn't it?

PS: In the mean time, I've prepared a small PR dagroe/test-ember-sourcemaps#2 to bundle the launch config you used in your previous post to help others test it

[dagroe]

Thanks for your PR :)

Works fine for me when using wagenet/ember-cli-htmlbars#fix-colocated-template-sourcemaps. Source maps look correct in browser debug (firefox and chrome) and work as expected in VSCode. Also making changes, reloading etc. seems to work fine.

[dagroe]

@chriskrycho @rwjblue @dfreeman sorry for the ping but I am not sure who to ask for a review of this. Could anyone of you do it or assign someone? Thanks!

[wagenet]

I just tried this is another app of my own and it appeared to drastically slow down the build. I'm not sure if the speed issue is actually related or if it's just a red herring. I'd like to try to get that figured out soon.

[dagroe]

That would be a big drawback 😞 , but it seems to make sense that generating sourcemaps with hires: true for all colocated components will slow down the build.

I was thinking that there might be a quicker way to generate the sourcemaps since source and generated files should be identical besides the two added lines for the template. I need to find some time to look into whether it makes sense to build the sourcemaps ourselves.

[dagroe]

Seems generating the source map is straight forward. I added it here: https://github.com/dagroe/ember-cli-htmlbars/tree/try-custom-sourcemap-generation @wagenet can you give this a try to see if it still works and improves build times in your project? 😄

[dagroe]

@wagenet Seems source maps are fixed in 6.2.0, at least for me they are working again without this PR.

[wagenet]

I took another pass at this and figured out a couple things. First, I fixed the paths so that source maps should fully work. (Note that because of unrelated issues in the build process, you may need to set them as "inline" in your babel config.)

As far as concerns about the build being slowed down, this seems to be due to a bad interaction with ember-template-imports v3 which parses all .js and .ts files. Somehow adding the source maps can end up being very bad for this.

[void-mAlex]

assuming didn't mean to introduce this

[wagenet]

Oops!

[dmyoung9]

@wagenet Seems we are not able to install your fork directly via npm, due to this line.

I was able to get it to work by cloning locally, removing that line, and installing it from my local filesystem, but this is not ideal.

Would it be possible to remedy this at the source?

EDIT: I can install it using pnpm, but this seems to cause issues with my other dependencies.

[dmyoung9]

I took another pass at this and figured out a couple things. First, I fixed the paths so that source maps should fully work. (Note that because of unrelated issues in the build process, you may need to set them as "inline" in your babel config.)

As far as concerns about the build being slowed down, this seems to be due to a bad interaction with ember-template-imports v3 which parses all .js and .ts files. Somehow adding the source maps can end up being very bad for this.

Unfortunately, using sourceMaps: 'inline' breaks debugging in Chrome DevTools... If I use just your fork, with no other modifications, debugging works in Chrome DevTools and code coverage is accurate, but I still haven't been able to get VS Code breakpoints to bind properly.