Merge pull request #7209 from weseek/fix/lsx-behavior

fix: Lsx behavior

packages/app/src/server/models/page.ts

@@ -196,10 +196,11 @@ export class PageQueryBuilder {
 
   /**
    * generate the query to find the pages '{path}/*' (exclude '{path}' self).
-   * If top page, return without doing anything.
    */
-  addConditionToListOnlyDescendants(path, option): PageQueryBuilder {
-    // No request is set for the top page
+  addConditionToListOnlyDescendants(path: string, option): PageQueryBuilder {
+    // exclude the target page
+    this.query = this.query.and({ path: { $ne: path } });
+
     if (isTopPage(path)) {
       return this;
     }
@@ -209,22 +210,24 @@ export class PageQueryBuilder {
     const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
 
     this.query = this.query
-      .and({ path: new RegExp(`^${startsPattern}`) });
+      .and(
+        { path: new RegExp(`^${startsPattern}`) },
+      );
 
     return this;
 
   }
 
-  addConditionToListOnlyAncestors(path): PageQueryBuilder {
+  addConditionToListOnlyAncestors(path: string): PageQueryBuilder {
     const pathNormalized = pathUtils.normalizePath(path);
     const ancestorsPaths = extractToAncestorsPaths(pathNormalized);
 
     this.query = this.query
-      .and({
-        path: {
-          $in: ancestorsPaths,
-        },
-      });
+      // exclude the target page
+      .and({ path: { $ne: path } })
+      .and(
+        { path: { $in: ancestorsPaths } },
+      );
 
     return this;
 

packages/remark-growi-directive/src/micromark-extension-growi-directive/lib/factory-attributes.js

@@ -7,9 +7,8 @@
 import { factorySpace } from 'micromark-factory-space';
 import { factoryWhitespace } from 'micromark-factory-whitespace';
 import {
-  asciiAlpha,
-  asciiAlphanumeric,
   markdownLineEnding,
+  markdownLineEndingOrSpace,
   markdownSpace,
 } from 'micromark-util-character';
 import { codes } from 'micromark-util-symbol/codes.js';
@@ -82,11 +81,16 @@ export function factoryAttributes(
       return shortcutStart(code);
     }
 
-    if (disallowEol && markdownSpace(code)) {
-      return factorySpace(effects, between, types.whitespace)(code);
+    if (disallowEol) {
+      if (markdownSpace(code)) {
+        return factorySpace(effects, between, types.whitespace)(code);
+      }
+      if (code === codes.comma) {
+        return factoryAttributesDevider(effects, between)(code);
+      }
     }
 
-    if (!disallowEol && (markdownLineEndingOrSpaceOrComma(code))) {
+    if (!disallowEol && markdownLineEndingOrSpaceOrComma(code)) {
       return factoryAttributesDevider(effects, between)(code);
     }
 
@@ -129,7 +133,7 @@ export function factoryAttributes(
       || code === codes.greaterThan
       || code === codes.graveAccent
       || code === codes.rightParenthesis
-      || markdownLineEndingOrSpaceOrComma(code)
+      || code === codes.comma
     ) {
       return nok(code);
     }
@@ -186,6 +190,7 @@ export function factoryAttributes(
         && code !== codes.graveAccent
         && code !== codes.rightParenthesis
         && code !== codes.space
+        && code !== codes.comma
     ) {
       effects.consume(code);
       return name;
@@ -197,7 +202,7 @@ export function factoryAttributes(
       return factorySpace(effects, nameAfter, types.whitespace)(code);
     }
 
-    if (!disallowEol && markdownLineEndingOrSpaceOrComma(code)) {
+    if (!disallowEol && markdownLineEndingOrSpace(code)) {
       return factoryAttributesDevider(effects, nameAfter)(code);
     }
 
@@ -245,7 +250,7 @@ export function factoryAttributes(
       return factorySpace(effects, valueBefore, types.whitespace)(code);
     }
 
-    if (!disallowEol && markdownLineEndingOrSpaceOrComma(code)) {
+    if (!disallowEol && markdownLineEndingOrSpace(code)) {
       return factoryAttributesDevider(effects, valueBefore)(code);
     }
 

packages/remark-growi-directive/test/micromark-extension-growi-directive.test.js

@@ -792,12 +792,16 @@ test('micromark-extension-directive (compile)', (t) => {
         'a $lsx()',
         'a $lsx(num=1)',
         'a $lsx(/)',
+        'a $lsx(/,num=5,depth=1)',
+        'a $lsx(/, num=5, depth=1)',
         'a $lsx(💚)',
         'Leaf:',
         '$lsx',
         '$lsx()',
         '$lsx(num=1)',
         '$lsx(/)',
+        '$lsx(/,num=5,depth=1)',
+        '$lsx(/, num=5, depth=1)',
         '$lsx(💚)',
       ].join('\n\n'),
       options({ lsx }),
@@ -808,12 +812,16 @@ test('micromark-extension-directive (compile)', (t) => {
       '<p>a <lsx ></lsx></p>',
       '<p>a <lsx num="1"></lsx></p>',
       '<p>a <lsx prefix="/"></lsx></p>',
+      '<p>a <lsx prefix="/" num="5" depth="1"></lsx></p>',
+      '<p>a <lsx prefix="/" num="5" depth="1"></lsx></p>',
       '<p>a <lsx prefix="💚"></lsx></p>',
       '<p>Leaf:</p>',
       '<lsx ></lsx>',
       '<lsx ></lsx>',
       '<lsx num="1"></lsx>',
       '<lsx prefix="/"></lsx>',
+      '<lsx prefix="/" num="5" depth="1"></lsx>',
+      '<lsx prefix="/" num="5" depth="1"></lsx>',
       '<lsx prefix="💚"></lsx>',
     ].join('\n'),
     'should support directives (lsx)',

packages/remark-lsx/src/server/routes/lsx.js

@@ -1,10 +1,15 @@
-const { customTagUtils } = require('@growi/core');
+import createError, { isHttpError } from 'http-errors';
+
+const { pagePathUtils, customTagUtils } = require('@growi/core');
 
 const { OptionParser } = customTagUtils;
 
 
 const DEFAULT_PAGES_NUM = 50;
 
+
+const { isTopPage } = pagePathUtils;
+
 class Lsx {
 
   /**
@@ -21,7 +26,7 @@ class Lsx {
   static addDepthCondition(query, pagePath, optionsDepth) {
     // when option strings is 'depth=', the option value is true
     if (optionsDepth == null || optionsDepth === true) {
-      throw new Error('The value of depth option is invalid.');
+      throw createError(400, 'The value of depth option is invalid.');
     }
 
     const range = OptionParser.parseRange(optionsDepth);
@@ -29,11 +34,13 @@ class Lsx {
     const end = range.end;
 
     if (start < 1 || end < 1) {
-      throw new Error(`specified depth is [${start}:${end}] : start and end are must be larger than 1`);
+      throw createError(400, `specified depth is [${start}:${end}] : start and end are must be larger than 1`);
     }
 
     // count slash
-    const slashNum = pagePath.split('/').length - 1;
+    const slashNum = isTopPage(pagePath)
+      ? 1
+      : pagePath.split('/').length;
     const depthStart = slashNum; // start is not affect to fetch page
     const depthEnd = slashNum + end - 1;
 
@@ -56,7 +63,7 @@ class Lsx {
   static addNumCondition(query, pagePath, optionsNum) {
     // when option strings is 'num=', the option value is true
     if (optionsNum == null || optionsNum === true) {
-      throw new Error('The value of num option is invalid.');
+      throw createError(400, 'The value of num option is invalid.');
     }
 
     if (typeof optionsNum === 'number') {
@@ -68,7 +75,7 @@ class Lsx {
     const end = range.end;
 
     if (start < 1 || end < 1) {
-      throw new Error(`specified num is [${start}:${end}] : start and end are must be larger than 1`);
+      throw createError(400, `specified num is [${start}:${end}] : start and end are must be larger than 1`);
     }
 
     const skip = start - 1;
@@ -92,7 +99,7 @@ class Lsx {
   static addFilterCondition(query, pagePath, optionsFilter, isExceptFilter = false) {
     // when option strings is 'filter=', the option value is true
     if (optionsFilter == null || optionsFilter === true) {
-      throw new Error('filter option require value in regular expression.');
+      throw createError(400, 'filter option require value in regular expression.');
     }
 
     let filterPath = '';
@@ -141,7 +148,7 @@ class Lsx {
     const isReversed = optionsReverse === 'true';
 
     if (optionsSort !== 'path' && optionsSort !== 'createdAt' && optionsSort !== 'updatedAt') {
-      throw new Error(`The specified value '${optionsSort}' for the sort option is invalid. It must be 'path', 'createdAt' or 'updatedAt'.`);
+      throw createError(400, `The specified value '${optionsSort}' for the sort option is invalid. It must be 'path', 'createdAt' or 'updatedAt'.`);
     }
 
     const sortOption = {};
@@ -163,26 +170,10 @@ module.exports = (crowi, app) => {
    * @return {Promise<Query>} query
    */
   async function generateBaseQueryBuilder(pagePath, user) {
-    let baseQuery = Page.find();
-    if (Page.PageQueryBuilder == null) {
-      if (Page.generateQueryToListWithDescendants != null) { // for Backward compatibility (<= GROWI v3.2.x)
-        baseQuery = Page.generateQueryToListWithDescendants(pagePath, user, {});
-      }
-      else if (Page.generateQueryToListByStartWith != null) { // for Backward compatibility (<= crowi-plus v2.0.7)
-        baseQuery = Page.generateQueryToListByStartWith(pagePath, user, {});
-      }
-
-      // return dummy PageQueryBuilder object
-      return Promise.resolve({ query: baseQuery });
-    }
+    const baseQuery = Page.find();
 
     const builder = new Page.PageQueryBuilder(baseQuery);
-    if (builder.addConditionToListOnlyDescendants == null) { // for Backward compatibility (<= GROWI v4.0.x)
-      builder.addConditionToListWithDescendants(pagePath);
-    }
-    else {
-      builder.addConditionToListOnlyDescendants(pagePath);
-    }
+    builder.addConditionToListOnlyDescendants(pagePath);
 
     builder
       .addConditionToExcludeTrashed();
@@ -245,6 +236,9 @@ module.exports = (crowi, app) => {
       res.status(200).send({ pages, toppageViewersCount });
     }
     catch (error) {
+      if (isHttpError) {
+        return res.status(error.status).send(error);
+      }
       return res.status(500).send(error);
     }
   };

packages/remark-lsx/src/stores/lsx.tsx

@@ -124,10 +124,12 @@ type LsxNodeTree = {
 export const useSWRxNodeTree = (lsxContext: LsxContext, isImmutable?: boolean): SWRResponse<LsxNodeTree, Error> => {
   const { data, error } = useSWRxLsxResponse(lsxContext.pagePath, lsxContext.options, isImmutable);
 
+  const isLoading = data === undefined && error == null;
+
   return useSWR(
-    data === undefined ? null : ['lsxNodeTree', lsxContext.pagePath, lsxContext.options, isImmutable, data],
+    !isLoading ? ['lsxNodeTree', lsxContext.pagePath, lsxContext.options, isImmutable, data, error] : null,
     (key, pagePath, options, isImmutable, data) => {
-      if (error != null) {
+      if (data === undefined || error != null) {
         throw error;
       }
       return {