Merge pull request #185 from PepsRyuu/PrettyFormatFrameError

Plugin warning formatting
PepsRyuu · Feb 27, 2021 · b53951f · b53951f
2 parents 7c1f00e + 9a4a017
commit b53951f
Show file tree

Hide file tree

Showing 7 changed files with 174 additions and 9 deletions.
diff --git a/examples/example-typescript/package.json b/examples/example-typescript/package.json
@@ -0,0 +1,15 @@
+{
+    "scripts": {
+        "clean": "shx rm -rf dist",
+        "start": "cross-env NODE_ENV=development node ../../lib/cli.js -c --hot --content-base public --port 9001",
+        "build": "npm run clean && cross-env NODE_ENV=production rollup -c"
+    },
+    "devDependencies": {
+        "@rollup/plugin-typescript": "^8.2.0",
+        "cross-env": "^5.2.0",
+        "rollup": "^1.28.0",
+        "shx": "^0.3.2",
+        "tslib": "^2.1.0",
+        "typescript": "^4.2.2"
+    }
+}
diff --git a/examples/example-typescript/public/index.html b/examples/example-typescript/public/index.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="UTF-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+    <title>Typescript Test</title>
+</head>
+<body>
+    <div id="app"></div>
+    <script async src="/main.[hash].js"></script>
+</body>
+</html>
diff --git a/examples/example-typescript/rollup.config.js b/examples/example-typescript/rollup.config.js
@@ -0,0 +1,16 @@
+import typescript from '@rollup/plugin-typescript';
+
+let config = {
+    input: './src/main.ts',
+    output: {
+        dir: 'dist',
+        format: 'esm',
+        entryFileNames: '[name].[hash].js',
+        assetFileNames: '[name].[hash][extname]'
+    },
+    plugins: [
+        typescript()
+    ]
+}
+
+export default config;
diff --git a/examples/example-typescript/src/main.ts b/examples/example-typescript/src/main.ts
@@ -0,0 +1,7 @@
+function add (a: number, b: number) {
+    return a + b;
+}
+
+let message: string = 'hello world';
+
+document.body.textContent = message + ', Adding 1 + 2 = ' + add(1, 2);
diff --git a/lib/impl/PluginContext.js b/lib/impl/PluginContext.js
@@ -137,7 +137,7 @@ module.exports = {
              * @param {string} e 
              */
             warn (e) {
-                console.warn(e);
+                container.__errorHandler.warn(e);
             },
 
             /**

diff --git a/lib/impl/PluginErrorHandler.js b/lib/impl/PluginErrorHandler.js
@@ -1,5 +1,37 @@
 // @ts-check
 
+// Formats the message as best it can.
+// Note that this diverges from Rollup warnings, which are formatted for the CLI only.
+// When using Rollup API by itself, it only prints normal warning message without all of the other properties like frame or position.
+// Nollup however has a dev-server, so it cannot take the CLI approach. Instead regardless of using CLI/API, it will be formatted.
+function format (error) {
+    let output = '';
+
+    if (typeof error === 'object') {
+        if (error.pluginCode) {
+            output += error.pluginCode + ': ';
+        }
+
+        if (error.message) {
+            output += error.message;
+        }
+
+        if (error.loc) {
+            output += '\n';
+            output += error.loc.file.replace(process.cwd(), '');
+            output += ` (${error.loc.line}:${error.loc.column})`;
+        }
+
+        if (error.frame) {
+            output += '\n' + error.frame;
+        }
+    } else {
+        output += error;
+    }    
+
+    return { message: output };
+}
+
 class PluginErrorHandler {
     /**
      * @param {function} callback 
@@ -12,18 +44,16 @@ class PluginErrorHandler {
         this.__errorThrown = false;
     }
 
+    warn (e) {
+        console.warn('\x1b[1m\x1b[33m' + format(e).message + '\x1b[39m\x1b[22m');
+    }
+
     /**
      * @param {object|string} e 
      * @return {void|never}
      */
     throw (e) {
-        if (typeof e === 'object' && e.frame) {
-            e.message = e.message + '\n' + e.frame;
-        }
-
-        if (typeof e === 'string') {
-            e = { message: e };
-        }
+        e = format(e);
 
         e.__isNollupError = true;
 

diff --git a/test/cases/api/context.js b/test/cases/api/context.js
@@ -527,7 +527,92 @@ describe ('API: Plugin Context', () => {
     });
 
     describe ('warn', () => {
-        it ('should output warning message');
+        let _consoleLog = console.warn;
+        let _logged = [];
+
+        beforeEach(() => {
+            console.warn = function (...args) {
+                _logged.push(args.join(' ').replace('\x1b[1m\x1b[33m', '').replace('\x1b[39m\x1b[22m', ''));
+            }
+        });
+
+        afterEach(() => {
+            console.warn = _consoleLog;
+            _logged = [];
+        });
+
+        it ('should output warning message', async () => {
+            fs.stub('./src/main.js', () => 'export default 123');
+
+            let bundle = await nollup({
+                input: './src/main.js',
+                plugins: [{
+                    transform () {
+                        this.warn('my warning');
+                    }
+                }]
+            });
+
+            await bundle.generate({ format: 'esm' });
+            expect(_logged[0]).to.equal('my warning');
+            fs.reset();
+        });
+
+        it ('should allow message object', async () => {
+            fs.stub('./src/main.js', () => 'export default 123');
+
+            let bundle = await nollup({
+                input: './src/main.js',
+                plugins: [{
+                    transform () {
+                        this.warn({ message: 'my warning' });
+                    }
+                }]
+            });
+
+            await bundle.generate({ format: 'esm' });
+            expect(_logged[0]).to.equal('my warning');
+            fs.reset();
+        });
+
+        it ('should allow message object with frame', async () => {
+            fs.stub('./src/main.js', () => 'export default 123');
+
+            let bundle = await nollup({
+                input: './src/main.js',
+                plugins: [{
+                    transform () {
+                        this.warn({ message: 'my warning', frame: '    var abc;\n    ^'});
+                    }
+                }]
+            });
+
+            await bundle.generate({ format: 'esm' });
+            expect(_logged[0]).to.equal('my warning\n    var abc;\n    ^');
+            fs.reset();
+        });
+
+        it ('should allow message object with pluginCode, message, loc and frame', async () => {
+            fs.stub('./src/main.js', () => 'export default 123');
+
+            let bundle = await nollup({
+                input: './src/main.js',
+                plugins: [{
+                    transform (code, id) {
+                        this.warn({ 
+                            message: 'my warning', 
+                            pluginCode: 'TS1234',
+                            loc: { line: 1, column: 5, file: id },
+                            frame: '    var abc;\n    ^'
+                        });
+                    }
+                }]
+            });
+
+            await bundle.generate({ format: 'esm' });
+            expect(_logged[0]).to.equal(`TS1234: my warning\n${path.resolve(process.cwd(), './src/main.js').replace(process.cwd(), '')} (1:5)\n    var abc;\n    ^`);
+            fs.reset();
+        });
     });
 
     describe ('error', () => {