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

Wrong line numbers when processing files after ts-loader #12025

Open
jeffrson opened this issue Nov 18, 2020 · 19 comments · May be fixed by #18260
Open

Wrong line numbers when processing files after ts-loader #12025

jeffrson opened this issue Nov 18, 2020 · 19 comments · May be fixed by #18260

Comments

@jeffrson
Copy link

Bug report

What is the current behavior?
Webpack in some cases reports errors according to .js files instead of originating .ts files. For the repro below the error is

ERROR in ./startup.ts 8:11-31
Module not found: Error: Can't resolve 'some_file' in 'C:\src\webpack-5\test'

Line number of error is 8 instead of 13 as in startup.ts - line 8 is the corresponding number from the .js file if package would be compiled with typescript.

If there would be a typescript error, the correct line of the error will be output.

If the current behavior is a bug, please provide the steps to reproduce.

https://github.com/jeffrson/webpack-ts-line-numbers.git

call yarn
then yarn webpack
finally check yarn tsc for comparing startup.js

What is the expected behavior?

should report line 13 and correct columns for file startup.ts

Other relevant information:
webpack version: 5.5.1
Node.js version: 14.15.1
Operating System: Windows 10
Additional tools:

@alexander-akait
Copy link
Member

alexander-akait commented Nov 18, 2020

I think it is expected, because resolving based on outputed file from loader, but yes, we can improve this, something like:

ERROR in ./startup.ts 8:11-31 (original 13:11-31)

@jeffrson
Copy link
Author

In this case it seems to be simple, but in a real project it's quite confusing if you look at the reported line and cannot find the cause.

In theory the transpiled .js could be reported (instead of .ts), but since this does not exist to have a look at for reference this is not quite helpful as well.

@alexander-akait
Copy link
Member

Yep, it will be great to improve this

@akgupta0777
Copy link

Assign it to me so that I can try to solve this.

@akgupta0777
Copy link

But which loader are you using to process typescript @jeffrson

@jeffrson
Copy link
Author

Just ts-loader

@akgupta0777
Copy link

ts-loader is a custom package created by TypeStrong. Its not a webpack owned loader in Webpack-contrib Github account.
I think I should go to ts-loader repo in order to fix this.

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

@alexander-akait
Copy link
Member

Still valid, we can get real position from source map, if they present

@lukepolo
Copy link

Is there a workaround ? Or do we need to wait for this to be completed?

@bonjourjoel
Copy link

ts-loader makes wrong source map line/column numbers.

@Zhen-dot
Copy link

Zhen-dot commented Jan 17, 2022

In case anyone comes across this issue and just wants a temporary workaround, I've added what works for me here

UPDATE A more user-friendly and perhaps more permanent solution here

@Anubarak
Copy link

Anubarak commented Nov 7, 2023

Since the other issue is closed does this mean there won't be any fix?
I have the same config as in the examples but the lines are still wrong - however I use Vue if that matters

@log101
Copy link

log101 commented Mar 21, 2024

I had a chance to work a bit on this issue. I'm able to get the original lines and locations of the error using the source map as @alexander-akait advised. I used source-map library for the conversion, but I wonder where is the right place to do it. For example it could be done here for the ModuleNotFoundError:

webpack/lib/Compilation.js

Lines 2094 to 2098 in 0bc85d1

const notFoundError = new ModuleNotFoundError(
originModule,
err,
dependencies.map(d => d.loc).filter(Boolean)[0]
);

I could update the dependency location (loc property), but it seems like this logic should be implemented for all errors.
So my idea is to do this conversion using the failedModule hook. Basically it'll check if the source map is available, if yes it'll do the conversion. Does this make sense?

@alexander-akait
Copy link
Member

I think we should do it after loader processing

@log101
Copy link

log101 commented Mar 25, 2024

I've been working on this problem since last week trying to find possible solutions. I'm still familiarizing myself with the codebase so I'm definitely open for feedback. Just to ensure I'm on the same page, I'd like to briefly describe the issue:
The problem is the misalignment of dependency locations. Misalignment occurs when a parent module is processed by loaders, changing line numbers. So when deps generate errors, the line numbers does not align with the original file, creating confusion. I have two potential solutions:

  1. As @alexander-akait suggested, converting the line numbers after loader processing. This could be implemented in the NormalModules's build function. handleParseResult function inside is called after module is built. Before sorting the deps at line 1109 we could add a mapper that'd convert the line numbers according to the original file using the source map. Such as:
// Check if source map is present
if (this.originalSource().map()) {
    this.dependencies.map(dep => convertLineNumbers(dep))
}

webpack/lib/NormalModule.js

Lines 1108 to 1120 in 0bc85d1

const handleParseResult = () => {
this.dependencies.sort(
concatComparators(
compareSelect(a => a.loc, compareLocations),
keepOriginalOrder(this.dependencies)
)
);
this._initBuildHash(compilation);
this._lastSuccessfulBuildMeta =
/** @type {BuildMeta} */
(this.buildMeta);
return handleBuildDone();
};

  1. Update the deps before ModuleNotFound error is thrown. But this would only fix the problem for one type of error. Before instantiating the ModuleNotFoundError class at line 2094, we could:
dependencies.map(dep => convertLineNumbers(dep))

webpack/lib/Compilation.js

Lines 2093 to 2100 in 0bc85d1

if (err) {
const notFoundError = new ModuleNotFoundError(
originModule,
err,
dependencies.map(d => d.loc).filter(Boolean)[0]
);
return callback(notFoundError, factoryResult ? result : undefined);
}

@alexander-akait
Copy link
Member

The first solution looks good, but let's it do only for errored modules

@log101
Copy link

log101 commented Mar 30, 2024

In order to confine the fix to errored modules only as @alexander-akait suggested, I deferred updating the error location to the module factorization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Priority - Low
Development

Successfully merging a pull request may close this issue.

9 participants