Repository: mermaid-js/mermaid
File: src/dagre-wrapper/nodes.js (JavaScript)

Code before:

  return shapeSvg;
};

const rect = (parent, node) => {
  const { shapeSvg, bbox, halfPadding } = labelHelper(parent, node, 'node ' + node.classes, true);

  log.trace('Classes = ', node.classes);
  // add the rect
  const rect = shapeSvg.insert('rect', ':first-child');

  const totalWidth = bbox.width + node.padding;
  const totalHeight = bbox.height + node.padding;
  rect
    .attr('class', 'basic label-container')
    .attr('style', node.style)
    .attr('rx', node.rx)
    .attr('ry', node.ry)
    .attr('x', -bbox.width / 2 - halfPadding)
    .attr('y', -bbox.height / 2 - halfPadding)
    .attr('width', totalWidth)
    .attr('height', totalHeight);

  const propKeys = new Set(Object.keys(node.props));
  if (node.props?.borders) {
    applyNodePropertyBorders(rect, node.props.borders, totalWidth, totalHeight);
    propKeys.delete('borders');
  }
  propKeys.forEach((propKey) => {
    log.warn(`Unknown node property ${propKey}`);
  });

  updateNodeBounds(node, rect);

  node.intersect = function (point) {
    return intersect.rect(node, point);
  };

  return shapeSvg;
};

function applyNodePropertyBorders(rect, borders, totalWidth, totalHeight) {
  const strokeDashArray = [];
  const addBorder = (length) => {
    strokeDashArray.push(length);
    strokeDashArray.push(0);
  };
  const skipBorder = (length) => {
    strokeDashArray.push(0);
    strokeDashArray.push(length);
  };

Review comment (refactor):
If properties are handled here in this function, only the required data has to be passed to each property handler (here `applyNodePropertyBorders`). I decided to not extract the whole block (handling all properties) as `applyNodeProperties` to have access to variables of the current function context (e.g. `totalWidth`). I could have passed them (e.g. `bbox`), but it is hard to predict, which variables may be of interest to future property handlers (e.g. that do something else than adding borders). I guess, all variables would be passed to `applyNodeProperties` in the end.

Code after:

  return shapeSvg;
};

const rect = (parent, node) => {
  const { shapeSvg, bbox, halfPadding } = labelHelper(parent, node, 'node ' + node.classes, true);

  log.trace('Classes = ', node.classes);
  // add the rect
  const rect = shapeSvg.insert('rect', ':first-child');

  const totalWidth = bbox.width + node.padding;
  const totalHeight = bbox.height + node.padding;
  rect
    .attr('class', 'basic label-container')
    .attr('style', node.style)
    .attr('rx', node.rx)
    .attr('ry', node.ry)
    .attr('x', -bbox.width / 2 - halfPadding)
    .attr('y', -bbox.height / 2 - halfPadding)
    .attr('width', totalWidth)
    .attr('height', totalHeight);

  if (node.props) {
    const propKeys = new Set(Object.keys(node.props));
    if (node.props.borders) {
      applyNodePropertyBorders(rect, node.props.borders, totalWidth, totalHeight);
      propKeys.delete('borders');
    }
    propKeys.forEach((propKey) => {
      log.warn(`Unknown node property ${propKey}`);
    });
  }

  updateNodeBounds(node, rect);

  node.intersect = function (point) {
    return intersect.rect(node, point);
  };

  return shapeSvg;
};

function applyNodePropertyBorders(rect, borders, totalWidth, totalHeight) {
  const strokeDashArray = [];
  const addBorder = (length) => {
    strokeDashArray.push(length);
    strokeDashArray.push(0);
  };
  const skipBorder = (length) => {
    strokeDashArray.push(0);