fix: styles rendering in case when "numFmt" is present in conditional…

… formatting rules (resolves exceljs#1814) (exceljs#1815) * fix: styles rendering in case when "numFmt" is present in conditional formatting rules (resolves exceljs#1814) * test: add integration test to cover exceljs#1814 * fix: lock jasmine version to prevent pipeline got stuck issue Co-authored-by: Andrii Krupskyi <[email protected]> Co-authored-by: Siemienik Pawel <[email protected]>
visnup · Sep 7, 2021 · f150952 · f150952
1 parent 78d9bda
commit f150952
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 19 deletions.
diff --git a/gruntfile.js b/gruntfile.js
@@ -27,14 +27,17 @@ module.exports = function(grunt) {
     browserify: {
       options: {
         transform: [
-          ['babelify', {
-            // enable babel transpile for node_modules
-            global: true,
-            presets: ['@babel/preset-env'],
-            // core-js should not be transpiled
-            // See https://github.com/zloirock/core-js/issues/514
-            ignore: [/node_modules[\\/]core-js/],
-          }],
+          [
+            'babelify',
+            {
+              // enable babel transpile for node_modules
+              global: true,
+              presets: ['@babel/preset-env'],
+              // core-js should not be transpiled
+              // See https://github.com/zloirock/core-js/issues/514
+              ignore: [/node_modules[\\/]core-js/],
+            },
+          ],
         ],
         browserifyOptions: {
           // enable source map for browserify
@@ -119,6 +122,10 @@ module.exports = function(grunt) {
     },
 
     jasmine: {
+      options: {
+        version: '3.8.0',
+        noSandbox: true,
+      },
       dev: {
         src: ['./dist/exceljs.js'],
         options: {

diff --git a/lib/xlsx/xform/style/dxf-xform.js b/lib/xlsx/xform/style/dxf-xform.js
@@ -39,8 +39,9 @@ class DxfXform extends BaseXform {
     if (model.font) {
       this.map.font.render(xmlStream, model.font);
     }
-    if (model.numFmt) {
-      this.map.numFmt.render(xmlStream, model.numFmt);
+    if (model.numFmt && model.numFmtId) {
+      const numFmtModel = {id: model.numFmtId, formatCode: model.numFmt};
+      this.map.numFmt.render(xmlStream, numFmtModel);
     }
     if (model.fill) {
       this.map.fill.render(xmlStream, model.fill);

diff --git a/lib/xlsx/xform/style/styles-xform.js b/lib/xlsx/xform/style/styles-xform.js
@@ -135,9 +135,7 @@ class StylesXform extends BaseXform {
       });
       xmlStream.closeNode();
 
-      this.map.cellStyleXfs.render(xmlStream, [
-        {numFmtId: 0, fontId: 0, fillId: 0, borderId: 0, xfId: 0},
-      ]);
+      this.map.cellStyleXfs.render(xmlStream, [{numFmtId: 0, fontId: 0, fillId: 0, borderId: 0, xfId: 0}]);
 
       xmlStream.openNode('cellXfs', {count: model.styles.length});
       model.styles.forEach(styleXml => {
@@ -150,9 +148,7 @@ class StylesXform extends BaseXform {
       this.map.fonts.render(xmlStream, model.fonts);
       this.map.fills.render(xmlStream, model.fills);
       this.map.borders.render(xmlStream, model.borders);
-      this.map.cellStyleXfs.render(xmlStream, [
-        {numFmtId: 0, fontId: 0, fillId: 0, borderId: 0, xfId: 0},
-      ]);
+      this.map.cellStyleXfs.render(xmlStream, [{numFmtId: 0, fontId: 0, fillId: 0, borderId: 0, xfId: 0}]);
       this.map.cellXfs.render(xmlStream, model.styles);
     }
 
@@ -313,8 +309,7 @@ class StylesXform extends BaseXform {
     // -------------------------------------------------------
     // number format
     if (style.numFmtId) {
-      const numFmt =
-        this.index.numFmt[style.numFmtId] || NumFmtXform.getDefaultFmtCode(style.numFmtId);
+      const numFmt = this.index.numFmt[style.numFmtId] || NumFmtXform.getDefaultFmtCode(style.numFmtId);
       if (numFmt) {
         model.numFmt = numFmt;
       }
@@ -349,6 +344,11 @@ class StylesXform extends BaseXform {
   }
 
   addDxfStyle(style) {
+    if (style.numFmt) {
+      // register numFmtId to use it during dxf-xform rendering
+      style.numFmtId = this._addNumFmtStr(style.numFmt);
+    }
+
     this.model.dxfs.push(style);
     return this.model.dxfs.length - 1;
   }

diff --git a/spec/integration/workbook-xlsx-writer/workbook-xlsx-writer.spec.js b/spec/integration/workbook-xlsx-writer/workbook-xlsx-writer.spec.js
@@ -588,5 +588,30 @@ describe('WorkbookWriter', () => {
           testUtils.checkTestBook(wb2, 'xlsx', ['conditionalFormatting']);
         });
     });
+
+    it('with conditional formatting that contains numFmt (#1814)', async () => {
+      const sheet = 'conditionalFormatting';
+      const options = {filename: TEST_XLSX_FILE_NAME, useStyles: true};
+
+      // generate file with conditional formatting that contains styles with numFmt
+      const wb1 = new ExcelJS.stream.xlsx.WorkbookWriter(options);
+      const ws1 = wb1.addWorksheet(sheet);
+      const cf1 = testUtils.conditionalFormatting.abbreviation;
+      ws1.addConditionalFormatting(cf1);
+      await wb1.commit();
+
+      // read generated file and extract saved conditional formatting rule
+      const wb2 = new ExcelJS.Workbook();
+      await wb2.xlsx.readFile(TEST_XLSX_FILE_NAME);
+      const ws2 = wb2.getWorksheet(sheet);
+      const [cf2] = ws2.conditionalFormattings;
+
+      // verify that rules from generated file contain styles with valid numFmt
+      cf2.rules.forEach(rule => {
+        expect(rule.style.numFmt).to.exist();
+        expect(rule.style.numFmt.id).to.be.a('number');
+        expect(rule.style.numFmt.formatCode).to.be.a('string');
+      });
+    });
   });
 });
diff --git a/spec/utils/data/conditional-formatting.json b/spec/utils/data/conditional-formatting.json
@@ -61,6 +61,35 @@
             }
         ]
     },
+    "abbreviation": {
+        "ref": "A:A",
+        "rules": [
+            {
+                "type": "cellIs",
+                "operator": "between",
+                "formulae": [1000, 1000000],
+                "style": { "numFmt": "#,##0.000,\\K;-#,##0.000,\\K" }
+            },
+            {
+                "type": "cellIs",
+                "operator": "between",
+                "formulae": [-1000, -1000000],
+                "style": { "numFmt": "#,##0.000,\\K;-#,##0.000,\\K" }
+            },
+            {
+                "type": "cellIs",
+                "operator": "greaterThan",
+                "formulae": [1000000],
+                "style": { "numFmt": "#,##0.000,,\\M;-#,##0.000,,\\M" }
+            },
+            {
+                "type": "cellIs",
+                "operator": "lessThan",
+                "formulae": [-1000000],
+                "style": { "numFmt": "#,##0.000,,\\M;-#,##0.000,,\\M" }
+            }
+        ]
+    },
     "types": [
         "expression",
         "cellIs",
@@ -71,4 +100,4 @@
         "containsText",
         "timePeriod"
     ]
-}
+}