Skip to content

Commit

Permalink
refactor: improving stack traces for included files, layouts & compon…
Browse files Browse the repository at this point in the history
…ents
  • Loading branch information
thetutlage committed Sep 14, 2019
1 parent 4c4300c commit 9b72583
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 36 deletions.
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"@poppinss/utils": "^2.0.0",
"debug": "^4.1.1",
"edge-error": "^1.0.3",
"edge-parser": "^2.0.10",
"edge-parser": "^2.1.0",
"he": "^1.2.0",
"import-fresh": "^3.1.0",
"lodash": "^4.17.15",
Expand Down
44 changes: 32 additions & 12 deletions src/Compiler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
*/

import { EdgeError } from 'edge-error'
import { Token, TagToken } from 'edge-lexer'
import { Parser, EdgeBuffer } from 'edge-parser'
import { Parser, EdgeBuffer, ParserToken, ParserTagToken } from 'edge-parser'

import { CacheManager } from '../CacheManager'
import { isBlockToken, getLineAndColumnForToken } from '../utils'
Expand All @@ -39,17 +38,22 @@ export class Compiler implements CompilerContract {
/**
* Merges sections of base template and parent template tokens
*/
private _mergeSections (base: Token[], extended: Token[], filename: string): Token[] {
private _mergeSections (
base: ParserToken[],
extended: ParserToken[],
filename: string,
layoutPath: string,
): ParserToken[] {
/**
* Collection of all sections from the extended tokens
*/
const extendedSections: { [key: string]: TagToken } = {}
const extendedSections: { [key: string]: ParserTagToken } = {}

/**
* Collection of extended set calls as top level nodes. Now since they are hanging
* up in the air, they will be hoisted like `var` statements in Javascript
*/
const extendedSetCalls: TagToken[] = []
const extendedSetCalls: ParserTagToken[] = []

extended
.forEach((node) => {
Expand Down Expand Up @@ -98,19 +102,24 @@ export class Compiler implements CompilerContract {
const finalNodes = base
.map((node) => {
if (!isBlockToken(node, 'section')) {
node.filename = layoutPath
return node
}

const extendedNode = extendedSections[node.properties.jsArg.trim()]
if (!extendedNode) {
node.filename = layoutPath
return node
}

/**
* Concat children when super was called
*/
if (extendedNode.children.length && isBlockToken(extendedNode.children[0], 'super')) {
extendedNode.children = node.children.concat(extendedNode.children)
extendedNode.children = node.children.map((child) => {
(child as ParserToken).filename = layoutPath
return child
}).concat(extendedNode.children)
}

return extendedNode
Expand All @@ -119,15 +128,19 @@ export class Compiler implements CompilerContract {
/**
* Set calls are hoisted to the top
*/
return ([] as Token[]).concat(extendedSetCalls).concat(finalNodes)
return ([] as ParserToken[]).concat(extendedSetCalls).concat(finalNodes)
}

/**
* Generates an array of lexer tokens from the template string. Further tokens
* are checked for layouts and if layouts are used, their sections will be
* merged together.
*/
private _templateContentToTokens (content: string, parser: Parser): Token[] {
private _templateContentToTokens (
content: string,
parser: Parser,
filename: string,
): ParserToken[] {
let templateTokens = parser.generateLexerTokens(content)
const firstToken = templateTokens[0]

Expand All @@ -137,13 +150,20 @@ export class Compiler implements CompilerContract {
*/
if (isBlockToken(firstToken, 'layout')) {
const layoutName = firstToken.properties.jsArg.replace(/'/g, '')

/**
* Making absolute path, so that lexer errors must point to the
* absolute file path
*/
const absPath = this._loader.makePath(layoutName)
const layoutTokens = this.generateLexerTokens(absPath)
templateTokens = this._mergeSections(layoutTokens, templateTokens, parser.options.filename)

templateTokens = this._mergeSections(
layoutTokens,
templateTokens,
filename,
absPath,
)
}

return templateTokens
Expand All @@ -158,11 +178,11 @@ export class Compiler implements CompilerContract {
* compiler.generateLexerTokens('<template-path>')
* ```
*/
public generateLexerTokens (templatePath: string): Token[] {
public generateLexerTokens (templatePath: string): ParserToken[] {
const { template } = this._loader.resolve(templatePath, false)

const parser = new Parser(this._tags, { filename: templatePath })
return this._templateContentToTokens(template, parser)
return this._templateContentToTokens(template, parser, templatePath)
}

/**
Expand Down Expand Up @@ -226,7 +246,7 @@ export class Compiler implements CompilerContract {
* Convert template to AST. The AST will have the layout actions merged (if layout)
* is used.
*/
const templateTokens = this._templateContentToTokens(template, parser)
const templateTokens = this._templateContentToTokens(template, parser, absPath)

/**
* Finally process the ast
Expand Down
8 changes: 4 additions & 4 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

import { sep } from 'path'
import { EdgeError } from 'edge-error'
import { Token, TagTypes, TagToken } from 'edge-lexer'
import { Parser, expressions as expressionsList } from 'edge-parser'
import { TagTypes } from 'edge-lexer'
import { Parser, expressions as expressionsList, ParserToken, ParserTagToken } from 'edge-parser'
import { StringifiedObject } from '../StringifiedObject'

/**
Expand Down Expand Up @@ -132,7 +132,7 @@ export function extractDiskAndTemplateName (templatePath: string): [string, stri
* Returns a boolean, telling whether the lexer node is a block node
* or not.
*/
export function isBlockToken (token: Token, name: string): token is TagToken {
export function isBlockToken (token: ParserToken, name: string): token is ParserTagToken {
if (token.type === TagTypes.TAG || token.type === TagTypes.ETAG) {
return token.properties.name === name
}
Expand All @@ -143,7 +143,7 @@ export function isBlockToken (token: Token, name: string): token is TagToken {
/**
* Returns line and number for a given AST token
*/
export function getLineAndColumnForToken (token: Token): [number, number] {
export function getLineAndColumnForToken (token: ParserToken): [number, number] {
if (token.type === 'newline' || token.type === 'raw') {
return [token.line, 0]
}
Expand Down
7 changes: 2 additions & 5 deletions test/edge.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,7 @@ test.group('Edge', (group) => {
}
})

/**
* Will fix it later
*/
test.skip('pass absolute path of layout to parser errors', async (assert) => {
test('pass absolute path of layout to parser errors', async (assert) => {
assert.plan(1)
await fs.add('foo.edge', `@layout('bar')`)
await fs.add('bar.edge', `{{ a:b }}`)
Expand All @@ -172,7 +169,7 @@ test.group('Edge', (group) => {
try {
edge.render('foo', false)
} catch ({ stack }) {
assert.equal(stack.split('\n')[1].trim(), `at (${join(fs.basePath, 'bar.edge')}:1:4)`)
assert.equal(stack.split('\n')[1].trim(), `at (${join(fs.basePath, 'bar.edge')}:1:3)`)
}
})

Expand Down
20 changes: 10 additions & 10 deletions test/tags.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ test.group('Include', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:2:13)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:2:13)`)
}
})

Expand All @@ -107,7 +107,7 @@ test.group('Include', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:1:9)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:1:9)`)
assert.equal(error.message, `{'foo', 'bar'} is not a valid argument type for the @include tag`)
}
})
Expand All @@ -132,7 +132,7 @@ test.group('Component', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:3:0)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:3:0)`)
assert.equal(error.message, '{hello} is not a valid argument type for the @slot tag')
}
})
Expand All @@ -151,7 +151,7 @@ test.group('Component', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:3:0)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:3:0)`)
assert.equal(error.message, 'maximum of 2 arguments are allowed for @slot tag')
}
})
Expand All @@ -170,7 +170,7 @@ test.group('Component', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:3:0)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:3:0)`)
assert.equal(error.message, 'slot name must be a valid string literal')
}
})
Expand All @@ -192,7 +192,7 @@ test.group('Component', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:5:6)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:5:6)`)
assert.equal(error.message, `{'props'} is not valid prop identifier for @slot tag`)
}
})
Expand All @@ -213,7 +213,7 @@ test.group('Component', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:4:12)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:4:12)`)
assert.equal(error.message, `@slot tag must appear as top level tag inside the @component tag`)
}
})
Expand Down Expand Up @@ -241,7 +241,7 @@ test.group('Layouts', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:3:8)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:3:8)`)
assert.equal(error.message, `Template extending the layout can only define @sections as top level nodes`)
}
})
Expand All @@ -264,7 +264,7 @@ test.group('Layouts', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:3:8)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:3:8)`)
assert.equal(error.message, `Template extending the layout can only define @sections as top level nodes`)
}
})
Expand All @@ -281,7 +281,7 @@ test.group('Layouts', (group) => {
try {
compiler.compile('foo', true)
} catch (error) {
assert.equal(error.stack.split('\n')[1], ` at (foo.edge:2:0)`)
assert.equal(error.stack.split('\n')[1], ` at (${join(fs.basePath, 'foo.edge')}:2:0)`)
assert.equal(error.message, `Template extending the layout can only define @sections as top level nodes`)
}
})
Expand Down

0 comments on commit 9b72583

Please sign in to comment.