prettier icon indicating copy to clipboard operation
prettier copied to clipboard

Comments in a ternary expression is unexpectedly split

Open liuxingbaoyu opened this issue 2 years ago • 3 comments

Prettier 2.8.8 Playground link

--parser babel

Input:

function buildNamespaceReexport() {
  return (
    constantReexports
      ? template.statement`
        Object.keys(NAMESPACE).forEach(function(key) {
          if (key === "default" || key === "__esModule") return;
          VERIFY_NAME_LIST;
          if (key in EXPORTS && EXPORTS[key] === NAMESPACE[key]) return;

          EXPORTS[key] = NAMESPACE[key];
        });
      `: // Also skip already assigned bindings if they are strictly equal
        // to be somewhat more spec-compliant when a file has multiple
        // namespace re-exports that would cause a binding to be exported
        // multiple times. However, multiple bindings of the same name that
        // export the same primitive value are silently skipped
        // (the spec requires an "ambiguous bindings" early error here).
        template.statement`
        Object.keys(NAMESPACE).forEach(function(key) {
          if (key === "default" || key === "__esModule") return;
          VERIFY_NAME_LIST;
          if (key in EXPORTS && EXPORTS[key] === NAMESPACE[key]) return;

          Object.defineProperty(EXPORTS, key, {
            enumerable: true,
            get: function() {
              return NAMESPACE[key];
            },
          });
        });
    `
  )({
    NAMESPACE: namespace,
    EXPORTS: metadata.exportName,
    VERIFY_NAME_LIST: metadata.exportNameListName
      ? template`
            if (EXPORTS_LIST.indexOf(key)) return;
          `({ EXPORTS_LIST: metadata.exportNameListName })
      : null,
  });
}


Output:

function buildNamespaceReexport() {
  return (
    constantReexports
      ? template.statement`
        Object.keys(NAMESPACE).forEach(function(key) {
          if (key === "default" || key === "__esModule") return;
          VERIFY_NAME_LIST;
          if (key in EXPORTS && EXPORTS[key] === NAMESPACE[key]) return;

          EXPORTS[key] = NAMESPACE[key];
        });
      ` // Also skip already assigned bindings if they are strictly equal
      : // to be somewhat more spec-compliant when a file has multiple
        // namespace re-exports that would cause a binding to be exported
        // multiple times. However, multiple bindings of the same name that
        // export the same primitive value are silently skipped
        // (the spec requires an "ambiguous bindings" early error here).
        template.statement`
        Object.keys(NAMESPACE).forEach(function(key) {
          if (key === "default" || key === "__esModule") return;
          VERIFY_NAME_LIST;
          if (key in EXPORTS && EXPORTS[key] === NAMESPACE[key]) return;

          Object.defineProperty(EXPORTS, key, {
            enumerable: true,
            get: function() {
              return NAMESPACE[key];
            },
          });
        });
    `
  )({
    NAMESPACE: namespace,
    EXPORTS: metadata.exportName,
    VERIFY_NAME_LIST: metadata.exportNameListName
      ? template`
            if (EXPORTS_LIST.indexOf(key)) return;
          `({ EXPORTS_LIST: metadata.exportNameListName })
      : null,
  });
}

Expected behavior:

We can see that the comment is wrongly split in two, if I add a newline before the : then it works.

liuxingbaoyu avatar Jun 17 '23 09:06 liuxingbaoyu

@liuxingbaoyu It seems v2.8.8 works the same way. I agree it's not expected, just not regression.

fisker avatar Jul 04 '23 03:07 fisker

You are right! I missed this.

liuxingbaoyu avatar Jul 04 '23 03:07 liuxingbaoyu

Updated to use stable verison of playground.

fisker avatar Jul 04 '23 03:07 fisker