v1.12.9.0: Fix incorrect error messages before libWrapper.Ready

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 1.12.9.0 (2022-09-11)
+
+- Fix incorrect error messages when calling the libWrapper API before the `libWrapper.Ready` hook. ([Issue  #72](https://github.com/ruipin/fvtt-lib-wrapper/issues/72))
+  - Add unit test to catch this issue in the future.
+- Fix accidental 3 consecutive blank lines in error messages when a module has neither an `url` nor a `bugs` entry in its manifest.
+- Update NPM dependencies to latest version.
+
 # 1.12.8.0 (2022-09-04)
 
 - Get rid of FVTT v10 deprecation warnings caused by legacy style package 'data' accesses.

README.md

@@ -312,7 +312,7 @@ To register a wrapper function, you should call the method `libWrapper.register(
  *
  * @param {string} package_id  The package identifier, i.e. the 'id' field in your module/system/world's manifest.
  *
- * @param {number|string} target The target identifier, specifying which wrapper should be unregistered.
+ * @param {number|string} target The target identifier, specifying which wrapper should be registered.
  *
  *   This can be either:
  *     1. A unique target identifier obtained from a previous 'libWrapper.register' call.

module.json

@@ -3,7 +3,7 @@
 	"name": "lib-wrapper",
 	"title": "libWrapper",
 	"description": "Library for wrapping core Foundry VTT methods, meant to improve compatibility between packages that wrap the same methods.",
-	"version": "1.12.8.0",
+	"version": "1.12.9.0",
 	"author": "Rui Pinheiro",
 	"authors": [{
 		"name": "Rui Pinheiro",

package.json

@@ -29,17 +29,15 @@
 		"deep-equal": "^2.0.5",
 		"lessc": "^1.0.2",
 		"npm-force-resolutions": "0.0.10",
-		"rollup": "^2.56.2",
+		"rollup": "^2.79.0",
 		"rollup-plugin-cleanup": "^3.2.1",
 		"rollup-plugin-jscc": "^2.0.0",
 		"rollup-plugin-terser": "^7.0.2",
 		"tap-dot": "^2.0.0",
 		"tap-spec": "^5.0.0",
-		"tape": "^5.3.1",
-		"tape-es": "^1.2.15"
+		"tape": "^5.6.0",
+		"tape-es": "^1.2.17"
 	},
 	"resolutions": {
-		"trim": "^0.0.3",
-		"terser": "^5.7.1"
 	}
 }

src/errors/base_errors.js

@@ -139,19 +139,19 @@ export class LibWrapperPackageError extends LibWrapperError {
 		let console_msg = `${console_ui_msg}\n\n${i18n.localize(`${key_prefix}.not-lw`)}\n\n`;
 
 		const info_url = package_info.url;
-		if(typeof info_url === 'string') {
+		const has_info = (typeof info_url === 'string');
+		if(has_info) {
 			console_msg += i18n.format(`${type_prefix}.info`, {type: pkg_type_i18n, url: info_url});
 		}
 
 		const report_url = package_info.bugs;
 		if(typeof report_url === 'string') {
-			console_msg += '\n';
 			console_msg += i18n.format(`${type_prefix}.report`, {url: report_url});
 		}
 		else {
 			const community_support_msg = this.get_community_support_message();
 			if(community_support_msg) {
-				console_msg += '\n\n';
+				if(has_info) console_msg += '\n\n';
 				console_msg += i18n.localize(`${key_prefix}.community-support`);
 				console_msg += '\n';
 				console_msg += community_support_msg;

src/lib/api.js

@@ -286,7 +286,7 @@ function _get_package_info(package_id) {
 	}
 	// Sanity Check: Package must exist (single exception is lib-wrapper, since we register wrappers before 'init')
 	else {
-		if(!package_info.exists && game.modules?.size)
+		if(!package_info.exists && globalThis.game?.modules?.size)
 			throw new ERRORS.package(`Package '${package_id}' is not a valid package.`, package_info);
 	}
 
@@ -343,7 +343,7 @@ export class libWrapper {
 	static get           AlreadyOverriddenError() { return ERRORS.already_overridden; };
 
 	static get LibWrapperInvalidWrapperChainError() { return ERRORS.invalid_chain; };
-	static get          InvalidWrapperChainError () { return ERRORS.invalid_chain; };
+	static get           InvalidWrapperChainError() { return ERRORS.invalid_chain; };
 
 	/* Undocumented on purpose, do not use */
 	static get onUnhandledError() { return onUnhandledError; };
@@ -385,7 +385,7 @@ export class libWrapper {
 	 *
 	 * @param {string} package_id  The package identifier, i.e. the 'id' field in your module/system/world's manifest.
 	 *
-	 * @param {number|string} target The target identifier, specifying which wrapper should be unregistered.
+	 * @param {number|string} target The target identifier, specifying which wrapper should be registered.
 	 *
 	 *   This can be either:
 	 *     1. A unique target identifier obtained from a previous 'libWrapper.register' call.
@@ -472,7 +472,7 @@ export class libWrapper {
 
 		// Validate we're allowed to register wrappers at this moment
 		if(package_id != PACKAGE_ID && !libwrapper_ready)
-			throw new ERRORS.package('Not allowed to register wrappers before the \'libWrapperReady\' hook fires', package_info);
+			throw new ERRORS.package('Not allowed to register wrappers before the \'libWrapper.Ready\' hook fires', package_info);
 
 		// Validate other arguments
 		if(typeof target !== 'string' && typeof target !== 'number')
@@ -619,6 +619,10 @@ export class libWrapper {
 		// Get package information
 		const package_info = _get_package_info(package_id);
 
+		// Validate we're allowed to unregister wrappers at this moment
+		if(package_id != PACKAGE_ID && !libwrapper_ready)
+			throw new ERRORS.package('Not allowed to unregister wrappers before the \'libWrapper.Ready\' hook fires', package_info);
+
 		// Validate arguments
 		if(typeof target !== 'string' && typeof target !== 'number')
 			throw new ERRORS.package('Parameter \'target\' must be a number or a string.', package_info);
@@ -649,6 +653,10 @@ export class libWrapper {
 		// Get package information
 		const package_info = _get_package_info(package_id);
 
+		// Validate we're allowed to unregister wrappers at this moment
+		if(package_id != PACKAGE_ID && !libwrapper_ready)
+			throw new ERRORS.package('Not allowed to unregister wrappers before the \'libWrapper.Ready\' hook fires', package_info);
+
 		// Clear package wrappers
 		WRAPPERS.forEach((wrapper) => {
 			this.unregister(package_info.id, wrapper.getter_id, false);
@@ -692,7 +700,7 @@ export class libWrapper {
 
 		// Validate we are allowed to call this method right now
 		if(!libwrapper_ready)
-			throw new ERRORS.package('Not allowed to ignore conflicts before the \'libWrapperReady\' hook fires', package_info);
+			throw new ERRORS.package('Not allowed to ignore conflicts before the \'libWrapper.Ready\' hook fires', package_info);
 
 		// Convert parameters to arrays
 		if(!Array.isArray(ignore_ids))
@@ -743,6 +751,7 @@ if(IS_UNITTEST) {
 	libWrapper._UT_clear_ignores              = (() => LibWrapperConflicts.clear_ignores());
 	libWrapper._UT_TGT_SPLIT_REGEX            = TGT_SPLIT_RE;
 	libWrapper._UT_TGT_CLEANUP_REGEX          = TGT_CLEANUP_RE;
+	libWrapper._UT_setReady                   = ((rdy) => libwrapper_ready = rdy);
 }
 Object.freeze(libWrapper);
 

tests/test_ignore.js

@@ -14,7 +14,7 @@ function setup() {
 	libWrapper._UT_unwrap_all();
 	libWrapper._UT_clear_ignores();
 
-	game.modules.clear();
+	game.reset();
 	globalThis.A = undefined;
 }
 

tests/test_lib.js

@@ -13,12 +13,49 @@ import {load_priorities} from '../src/ui/settings.js';
 
 function setup() {
 	libWrapper._UT_unwrap_all();
+	libWrapper._UT_setReady(true);
 	load_priorities();
-
-	game.clear_modules();
+	game.reset();
 }
 
 
+// Pre-ready functionality
+test_combinations('Library: Pre-ready', async function(t) {
+	setup();
+
+	// Set libWrapper to not-ready, and mimic a non-initialised game
+	libWrapper._UT_setReady(false);
+	game.modules = undefined;
+	game.ready = false;
+
+	// Check that we throw when libWrapper is not yet ready
+	t.throws(
+		() => libWrapper.register('some-module', 'A.prototype.x', () => {}, 'MIXED'),
+		new RegExp("Not allowed to register wrappers before the 'libWrapper.Ready' hook fires"),
+		"Calling 'register' before ready should throw"
+	);
+
+	t.throws(
+		() => libWrapper.unregister('some-module', 'A.prototype.x'),
+		new RegExp("Not allowed to unregister wrappers before the 'libWrapper.Ready' hook fires"),
+		"Calling 'unregister' before ready should throw"
+	);
+
+	t.throws(
+		() => libWrapper.unregister_all('some-module'),
+		new RegExp("Not allowed to unregister wrappers before the 'libWrapper.Ready' hook fires"),
+		"Calling 'unregister_all' before ready should throw"
+	);
+
+	t.throws(
+		() => libWrapper.ignore_conflicts('some-module', 'another-module', 'A.prototype.x'),
+		new RegExp("Not allowed to ignore conflicts before the 'libWrapper.Ready' hook fires"),
+		"Calling 'ignore_conflicts' before ready should throw"
+	);
+
+	t.end();
+});
+
 
 // Main functionality of libWrapper
 test_combinations('Library: Main', async function (t) {
@@ -42,9 +79,11 @@ test_combinations('Library: Main', async function (t) {
 	await chkr.call(a, 'x', ['m1:Mix:1','Orig',-2]);
 
 	// Registering the same method twice with the same module should fail
-	t.throws(function() {
-		libWrapper.register('m1', 'A.prototype.x', () => {});
-	}, libWrapper.Error, 'Registering twice with same module should fail');
+	t.throws(
+		() => libWrapper.register('m1', 'A.prototype.x', () => {}),
+		libWrapper.Error,
+		'Registering twice with same module should fail'
+	);
 	await chkr.call(a, 'x', ['m1:Mix:1','Orig',-2]);
 
 	// Register WRAPPER
@@ -59,9 +98,11 @@ test_combinations('Library: Main', async function (t) {
 
 	// Registing another OVERRIDE should fail
 	game.add_module('m4');
-	t.throws(function() {
-		libWrapper.register('m4', 'A.prototype.x', () => {}, 'OVERRIDE');
-	}, libWrapper.AlreadyOverriddenError, 'Registering second override should fail');
+	t.throws(
+		() => libWrapper.register('m4', 'A.prototype.x', () => {}, 'OVERRIDE'),
+		libWrapper.AlreadyOverriddenError,
+		'Registering second override should fail'
+	);
 	await chkr.call(a, 'x', ['m2:Wrp:2','m1:Mix:1','m3:Ovr:3',-3]);
 
 	// Unless the module has a higher priority
@@ -126,11 +167,11 @@ test_combinations('Library: Main', async function (t) {
 
 
 	// Test invalid getter
-	t.throws(() => libWrapper.register('m1', 'A.prototype.xyz', ()=>{}), libWrapper.ModuleError, "Wrap invalid getter");
+	t.throws(() => libWrapper.register('m1', 'A.prototype.xyz', ()=>{}), libWrapper.PackageError, "Wrap invalid getter");
 
 	// Test invalid setter
-	t.throws(() => libWrapper.register('m1', 'A.prototype.x#set', ()=>{}), libWrapper.ModuleError, "Wrap invalid setter");
-	t.throws(() => libWrapper.register('m1', 'A.prototype.xyz#set', ()=>{}), libWrapper.ModuleError, "Wrap invalid setter");
+	t.throws(() => libWrapper.register('m1', 'A.prototype.x#set', ()=>{}), libWrapper.PackageError, "Wrap invalid setter");
+	t.throws(() => libWrapper.register('m1', 'A.prototype.xyz#set', ()=>{}), libWrapper.PackageError, "Wrap invalid setter");
 
 
 	// Done

tests/test_performance.js

@@ -14,7 +14,8 @@ import '../src/lib/api.js';
 
 function setup() {
 	libWrapper._UT_unwrap_all();
-	game.clear_modules();
+
+	game.reset();
 }
 
 

tests/test_shim.js

@@ -19,7 +19,7 @@ import '../src/lib/api.js';
 function setup() {
 	libWrapper._UT_unwrap_all();
 
-	game.clear_modules();
+	game.reset();
 }
 
 

tests/test_wrapper.js

@@ -11,7 +11,7 @@ import '../src/lib/api.js';
 function setup() {
 	libWrapper._UT_unwrap_all();
 
-	game.modules.clear();
+	game.reset();
 	globalThis.A = undefined;
 }
 

tests/utilities.js

@@ -65,7 +65,10 @@ class GameSettings {
 
 class Game {
 	constructor() {
-		this.modules = new Map();
+		this.reset();
+	}
+
+	reset() {
 		this.settings = new GameSettings();
 		this.user = { isGM: true, userId: 12345 };
 		this.userId = 12345;
@@ -85,6 +88,13 @@ class Game {
 			}
 		}
 		this.ready = true;
+
+		this.clear_modules();
+	}
+
+	clear_modules() {
+		this.modules = new Map();
+		this.add_module('lib-wrapper');
 	}
 
 	add_module(nm) {
@@ -95,12 +105,7 @@ class Game {
 		else
 			mdl.data = { title: nm };
 
-		game.modules.set(nm, mdl);
-	}
-
-	clear_modules() {
-		this.modules.clear();
-		this.add_module('lib-wrapper');
+		this.modules.set(nm, mdl);
 	}
 }
 globalThis.game = new Game();