diagnostics throws error if a notfound route is within a Private tag

[M0nica]

This should resolve (Add error diagnostic: notfound page cannot be private) #920.

I tested it locally by running a test app alongside the redwood instance and checking that an error was only thrown when running the diagnostics command when a Private tag contained a Route with the notfound attribute.

Below is a screenshot of how the error renders for me when running yarn rw check

[M0nica]

references to isAuthenticated should be removed in #917 there's a comment by @aldonline indicating that references to authenticated are no longer being used in favor of private however, it can't be removed safely without modifying the outline file which is still referencing isAuthenticated.

You'll notice that there is some old code/stub that references a property called route.isAuthenticated. The "isAuthenticated" name is from before the "Private" tag name was chosen.

[aldonline]

Yeah. We need to clean that up :)

[M0nica]

This was the best way I could figure out how to access the tagName for parent as the methods available to jsxNode aren't available for the parent node but I am open to suggestions re: different approaches to determining if the parent node is a Private tag.

[aldonline]

Hi Monica,

(edit)

1. I'm not 100% sure, but I think this heuristic might break if there is trivia before the <Private> tag (like a comment). The getFullText() method might return the trivia as well. Please be sure to check this.
2. Once you've found a good strategy to test for this, please extract it to a separate getter @lazy() get isPrivate(): boolean {...}. It is important to keep all these heuristics consistent. We don't want some algorithms to use your strategy, and then someone else reinventing the wheel.

FYI: the ts-morph way of doing this would be something like this:

    const tagText = this.jsxNode
      .getParentIfKind(tsm.SyntaxKind.JsxElement)
      ?.getOpeningElement()
      ?.getTagNameNode()
      ?.getText()

[M0nica]

Thanks for the feedback. I refactored the code based on your feedback and it worked both with and without a comment infront of the tag from my manual testing.

[aldonline]

Thanks for your PR!
I added a few comments suggesting changes ;)

[aldonline]

Suggested change

	if (typeof this.jsxNode.getAttribute('notfound') !== 'undefined') {
	if (this.isNotFound) {

[aldonline]

Hi Monica,

(edit)

1. I'm not 100% sure, but I think this heuristic might break if there is trivia before the <Private> tag (like a comment). The getFullText() method might return the trivia as well. Please be sure to check this.
2. Once you've found a good strategy to test for this, please extract it to a separate getter @lazy() get isPrivate(): boolean {...}. It is important to keep all these heuristics consistent. We don't want some algorithms to use your strategy, and then someone else reinventing the wheel.

FYI: the ts-morph way of doing this would be something like this:

    const tagText = this.jsxNode
      .getParentIfKind(tsm.SyntaxKind.JsxElement)
      ?.getOpeningElement()
      ?.getTagNameNode()
      ?.getText()

[aldonline]

Yeah. We need to clean that up :)

[thedavidprice]

Hi @M0nica excited to see others helping out with the structure package! You're in good hands with @aldonline who's been teaching us all the ins and outs about how to do this.

[aldonline]

Thanks @M0nica!
I think this is ok to merge now (pending rebasing to "main", but let's wait for @thedavidprice or @peterp to suggest if that's necessary)
It also works in the IDE ;)

[peterp]

Love it, thanks!