Skip to content
This repository has been archived by the owner on May 1, 2019. It is now read-only.

properly set sourceRange and astNode on inline documents #480

Merged
merged 3 commits into from Feb 23, 2017

Conversation

FredKSchott
Copy link
Contributor

  • Quick-ish fix to properly set sourceRange and astNode on inlined documents.
  • Add TODO to document a better fix for when we have time

/cc @rictic @justinfagnani

// not have to be set awkwardly after the fact.
sourceRange: SourceRange|undefined = undefined;
astNode: dom5.Node|undefined = undefined;
;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extra semicolon

const jsDoc = jsDocs.values().next().value;
assert.deepEqual(jsDoc.sourceRange, {
file: 'static/inline-documents/inline-documents.html',
start: {line: 3, column: 2},
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the CodeUnderliner rather than doing equality checking against source range

assert.equal(cssDocs.size, 1);
const cssDoc = cssDocs.values().next().value;
assert.deepEqual(cssDoc.sourceRange, {
file: 'static/inline-documents/inline-documents.html',
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

});
assert.isObject(jsDoc.astNode);
assert.equal(jsDoc.astNode!.tagName, 'script');
assert.equal(jsDoc.astNode!.parentNode!.tagName, 'head');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the assertion on the parent node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chose it as a simple proxy for checking that the astNode was accurate. But having worked with the analyzer more now I feel we can assume that if the astNode is set it is accurate (at least in a larger analyzer test like this). So I'll back this out.

@FredKSchott FredKSchott merged commit de3b588 into master Feb 23, 2017
@FredKSchott FredKSchott deleted the inline-documents-sourcerange branch February 23, 2017 23:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants