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

Fix line numbers in source maps and stack traces #296

Merged

Conversation

rahul-mahato
Copy link
Contributor

@rahul-mahato rahul-mahato commented Feb 15, 2021

Fix #259

})
return lineOfExport
}
function getLineOfRVOptions(content) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines should be left between functions

Suggested change
function getLineOfRVOptions(content) {
function getLineOfReactVueOptions(content) {

function getLineOfExport(content) {
var lineOfExport = 0
content.split(newLine).some((line, index) => {
if (line.includes('export')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may also match the string export, if it is inside a string and not being used as a keyword. Could we check this in a better way?

var exportDefaultIndex = code.indexOf('export default')
var tempString = code.substring(0, exportDefaultIndex)
var exportDefaultLineNumber = tempString.split('\n').length
var scriptTagLineNumber = getLineOfExport(parsedSFC.script.content)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is code outside the export block, after the imports? We need to test for this

@RishabhKarnad RishabhKarnad changed the title Fix/source map line numbers Fix line numbers in source maps and stack traces Feb 15, 2021
return firstLine
}

function getFirstNonLineOfText(content, text) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function getFirstNonLineOfText(content, text) {
function getFirstLineWithoutText(content, text) {

@@ -248,3 +271,46 @@ function traverse(ast, nodes = []) {
})
}
}

function getFirstLineOfText(content, text) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function getFirstLineOfText(content, text) {
function getFirstLineWithText(content, text) {

@@ -248,3 +271,46 @@ function traverse(ast, nodes = []) {
})
}
}

function getFirstLineOfText(content, text) {
var firstLine = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var firstLine = 0
let firstLine = 0


function getFirstLineOfText(content, text) {
var firstLine = 0
let splittedContent = content.split(newLine)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let splittedContent = content.split(newLine)
let contentByLine = content.split(newLine)

if (!content.includes(text)) {
return splittedContent.length
}
splittedContent.some((line, index) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is confusing. some should be used to check for membership in an array.

Would be better to use findIndex

Would also remove the need for the additional variable firstLine

}

function getFirstNonLineOfText(content, text) {
var firstNonCommentedLine = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var firstNonCommentedLine = 0
let firstNonCommentedLine = 0

Use const for declarations and let only if there is a need to mutate. Avoid var

if (!content.includes(text)) {
return 0
}
splittedContent.some((line, index) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is confusing. some should be used to check for membership in an array.

Would be better to use findIndex

Would also remove the need for the additional variable firstNonCommentedLine

@RishabhKarnad RishabhKarnad mentioned this pull request Feb 21, 2021
@RishabhKarnad RishabhKarnad merged commit 5b40b3f into GeekyAnts:develop Mar 4, 2021
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.

None yet

2 participants