Simplify baseUrl resolving process #1087
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ const nunjucks = require('nunjucks'); | |
| const path = require('path'); | ||
| const Promise = require('bluebird'); | ||
| const slugify = require('@sindresorhus/slugify'); | ||
| const ensurePosix = require('ensure-posix-path'); | ||
| const componentParser = require('./parsers/componentParser'); | ||
| const componentPreprocessor = require('./preprocessors/componentPreprocessor'); | ||
| const nunjuckUtils = require('./utils/nunjuckUtils'); | ||
|
|
@@ -139,8 +140,8 @@ class Parser { | |
| return utils.createErrorNode(node, e); | ||
| } | ||
| const fileContent = this._fileCache[filePath]; // cache the file contents to save some I/O | ||
| const parentSitePath = urlUtils.getParentSiteAbsolutePath(asIfAt, config.rootPath, config.baseUrlMap); | ||
| const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; | ||
| // Extract included variables from the PARENT file | ||
| const includeVariables = Parser.extractIncludeVariables(node, context.variables); | ||
| // Extract page variables from the CHILD file | ||
|
|
@@ -395,9 +396,8 @@ class Parser { | |
| reject(err); | ||
| return; | ||
| } | ||
| const parentSitePath = urlUtils.getParentSiteAbsolutePath(file, config.rootPath, config.baseUrlMap); | ||
| const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; | ||
| const pageVariables = this.extractPageVariables(file, data, userDefinedVariables, {}); | ||
| let fileContent = nunjuckUtils.renderEscaped(nunjucks, data, { | ||
| ...pageVariables, | ||
|
|
@@ -466,9 +466,8 @@ class Parser { | |
| decodeEntities: true, | ||
| }); | ||
|
|
||
| const parentSitePath = urlUtils.getParentSiteAbsolutePath(file, config.rootPath, config.baseUrlMap); | ||
| const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; | ||
| const { additionalVariables } = config; | ||
| const pageVariables = this.extractPageVariables(actualFilePath, pageData, userDefinedVariables, {}); | ||
|
|
||
|
|
@@ -555,30 +554,17 @@ class Parser { | |
| } | ||
|
|
||
| resolveBaseUrl(pageData, config) { | ||
| const { baseUrlMap, rootPath } = config; | ||
| this.baseUrlMap = baseUrlMap; | ||
| this.rootPath = rootPath; | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| const handler = new htmlparser.DomHandler((error, dom) => { | ||
| if (error) { | ||
| reject(error); | ||
| return; | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. see the comment for |
||
| const nodes = dom.map(d => this._rebaseReference(d)); | ||
| cheerio.prototype.options.xmlMode = false; | ||
| resolve(cheerio.html(nodes)); | ||
| cheerio.prototype.options.xmlMode = true; | ||
|
|
@@ -591,53 +577,51 @@ class Parser { | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Pre-renders the baseUrl of the provided node and its children according to | ||
| * the site they belong to, such that the final call to render baseUrl correctly | ||
| * resolves baseUrl according to the said site. | ||
| */ | ||
| _rebaseReference(node) { | ||
|
Contributor
Author
There was a problem hiding this comment. Two changes here essentially:
|
||
| if (_.isArray(node)) { | ||
| return node.map(el => this._rebaseReference(el)); | ||
| } | ||
| if (Parser.isText(node)) { | ||
| return node; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. This seems to be the PR that added the
Contributor
Author
There was a problem hiding this comment. Thanks for this! Unfortunately the PR dosen't really reveal the workings of
Going by this, it should be sufficient to resolve the baseurl only when the include's file and included file belongs to different sites. The likely cause of the issue was wrongly resolving against I was looking at 64b2ff6 previously, where |
||
|
|
||
| // Rebase children elements | ||
| node.children.forEach(el => this._rebaseReference(el)); | ||
|
|
||
| const includeSourceFile = node.attribs[ATTRIB_CWF]; | ||
| const includedSourceFile = node.attribs[ATTRIB_INCLUDE_PATH]; | ||
| delete node.attribs[ATTRIB_CWF]; | ||
|
Contributor
There was a problem hiding this comment. Just wondering - should we delete these attributes here? It feels a little odd since they were added in If it is necessary for us to delete it here, could we leave a comment explaining why we need to do this? And maybe add a comment in
Contributor
Author
There was a problem hiding this comment. The original function already deletes both
Contributor
Author
There was a problem hiding this comment. It's not necessary to the best of my knowledge, but not having
Contributor
There was a problem hiding this comment. Ah noted I didn't notice that these outputs would be included in the output! |
||
| delete node.attribs[ATTRIB_INCLUDE_PATH]; | ||
|
|
||
| // Skip rebasing for non-includes | ||
| if (!includedSourceFile || !node.children) { | ||
| return node; | ||
| } | ||
|
|
||
| // The site at which the file with the include element was pre-processed | ||
| const currentBase = urlUtils.getParentSiteAbsolutePath(includeSourceFile, this.rootPath, this.baseUrlMap); | ||
| // The site of the included file | ||
| const newBase = urlUtils.getParentSiteAbsoluteAndRelativePaths(includedSourceFile, this.rootPath, | ||
| this.baseUrlMap); | ||
|
|
||
| // Only re-render if include src and content are from different sites | ||
| if (currentBase !== newBase.absolute) { | ||
| cheerio.prototype.options.xmlMode = false; | ||
| const rendered = nunjuckUtils.renderEscaped(nunjucks, cheerio.html(node.children), { | ||
| // This is to prevent the nunjuck call from converting {{hostBaseUrl}} to an empty string | ||
| // and let the hostBaseUrl value be injected later. | ||
| hostBaseUrl: '{{hostBaseUrl}}', | ||
| baseUrl: `{{hostBaseUrl}}/${ensurePosix(newBase.relative)}`, | ||
| }, { path: includedSourceFile }); | ||
| node.children = cheerio.parseHTML(rendered, true); | ||
| cheerio.prototype.options.xmlMode = true; | ||
| } | ||
|
|
||
| return node; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these variables in
resolveDependencyare not necessary, as in theresolveBaseUrlprocess,attribs[ATTRIB_CWF] = context.cwfis the same asdependency.from( ie.parser.dynamicIncludeSrc.push({ from: context.cwf, ... }))