From 9b7258301daa3c37320b416155bbc41f4d73c09c Mon Sep 17 00:00:00 2001 From: Harminder virk Date: Sat, 14 Sep 2019 21:04:57 +0530 Subject: [PATCH] refactor: improving stack traces for included files, layouts & components --- package-lock.json | 8 ++++---- package.json | 2 +- src/Compiler/index.ts | 44 +++++++++++++++++++++++++++++++------------ src/utils/index.ts | 8 ++++---- test/edge.spec.ts | 7 ++----- test/tags.spec.ts | 20 ++++++++++---------- 6 files changed, 53 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index 67f4e0a..6462d61 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "edge", + "name": "edge.js", "version": "2.0.0", "lockfileVersion": 1, "requires": true, @@ -1698,9 +1698,9 @@ } }, "edge-parser": { - "version": "2.0.10", - "resolved": "https://registry.npmjs.org/edge-parser/-/edge-parser-2.0.10.tgz", - "integrity": "sha512-wFewug2VOhPfgtnw2QMYhJRIinDENx745Wm7scja/YFRF8xLM68KCdmRYXqWRD+QaUAD8mjymTlWHGEoGveESw==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/edge-parser/-/edge-parser-2.1.0.tgz", + "integrity": "sha512-WMPwr2D0U7SMppRAgZoZdDYqsABe0wTU/WMynAJ4e24hKr7+cx7Y653kSy+5/f3muaI5YCSeBcS+6VEoqeoOcQ==", "requires": { "acorn": "^7.0.0", "astring": "^1.4.1", diff --git a/package.json b/package.json index becdc8b..148f8ca 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/Compiler/index.ts b/src/Compiler/index.ts index b8ece52..f45117a 100644 --- a/src/Compiler/index.ts +++ b/src/Compiler/index.ts @@ -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' @@ -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) => { @@ -98,11 +102,13 @@ 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 } @@ -110,7 +116,10 @@ export class Compiler implements CompilerContract { * 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 @@ -119,7 +128,7 @@ 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) } /** @@ -127,7 +136,11 @@ export class Compiler implements CompilerContract { * 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] @@ -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 @@ -158,11 +178,11 @@ export class Compiler implements CompilerContract { * compiler.generateLexerTokens('') * ``` */ - 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) } /** @@ -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 diff --git a/src/utils/index.ts b/src/utils/index.ts index d862160..0fb0524 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -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' /** @@ -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 } @@ -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] } diff --git a/test/edge.spec.ts b/test/edge.spec.ts index 486b742..1e61955 100644 --- a/test/edge.spec.ts +++ b/test/edge.spec.ts @@ -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 }}`) @@ -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)`) } }) diff --git a/test/tags.spec.ts b/test/tags.spec.ts index 019d02d..fd64250 100644 --- a/test/tags.spec.ts +++ b/test/tags.spec.ts @@ -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)`) } }) @@ -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`) } }) @@ -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') } }) @@ -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') } }) @@ -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') } }) @@ -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`) } }) @@ -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`) } }) @@ -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`) } }) @@ -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`) } }) @@ -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`) } })