-
Notifications
You must be signed in to change notification settings - Fork 643
Description
When including objection.js in a bundle that is minified I get an exception like the following [Note: some other minified identifier may appear in the place of Fj]:
TypeError: Class constructor Fj cannot be invoked without 'new'
In my case, the error was thrown when using joinRelated() on a model, though after tracking down the issue there may be other cases that would also cause problems.
For a stand-alone example that replicates the error, and additional details on the issue see this repo. I'd be happy to submit a PR with the solution proposed in 3(ii) below if desired (it solved my issue when applied in a fork).
Explanation of the issue
The issue appears to be in knexUtils.js (lines 39-39)
Notice that the checks for isKnexQueryBuilder, isKnexJoinBuilder, and isKnexRaw look for a specific constructor using a literal string value. This comparison is bound to fail when the identifiers being compared against have been minified.
Potential solutions/work-arounds
-
The first solution is to avoid minifying the code, with the obvious downside being a larger bundle size. Of course the impact will vary depending on the project, but for this example (where the majority of the code is objection.js and knex) the unminified bundle is twice the size of the minified bundle.
-
The second solution is to partially minify the code by utilizing the more granular
--minify-whitespaceand--minify-syntaxoptions of esbuild instead of the more general--minifyoption (i.e. avoiding identifier minification which can be seperately controlled via the--minify-identifiersoption). This will avoid the issue while still providing some reduction in the size of the bundle (though less reduction than with full minification). -
The third possible solution is to modify the code in objection.js so that a string literal is not used to complete the checks within
isKnexQueryBuilder,isKnexJoinBuilder, andisKnexRawfound inknexUtils.js. Here are three potential modifications that could be made:-
The obvious solution is to utilize
instanceofto see if the variable is an instance of the appropriate class. However, this is not possible because the classes of interest are internal to knex and are not publicly exported. So, the first potential modification is to modify knex so that theBuilder,JoinClauseandRawclasses are exported publicly, so that the appropriate identifier is used even when minified. Then the objection.jsknexUtils.jschecks could be modified similar to:import { JoinClause } from 'knex' function isKnexJoinBuilder(value) { return value instanceof JoinClause; }
Note that this requires changes within both knex and objection.js
-
The next possibility restricts changes to within the objection.js file
knexUtils.jswhere theisKnexQueryBuilder,isKnexJoinBuilder, andisKnexRawfunctions could utilize unique properties of each knex class to identify instances of each. While it may not be required, you could also still ensure that each has a constructor (just not a specific constructor), e.g.function hasConstructor(value) { return isObject(value) && isFunction(value.constructor); }
And then check for the existence of fields unique to each class, e.g.
function isKnexQueryBuilder(value) { return ( hasConstructor(value) && isFunction(value.select) && isFunction(value.column) && value.select === value.column && 'client' in value ); }
See knex/lib/query/querybuilder.js:L1452-L1453
function isKnexJoinBuilder(value) { return hasConstructor(value) && value.grouping === 'join' && 'joinType' in value; }
See knex/lib/query/joinclause.js:L237-L239
function isKnexRaw(value) { return hasConstructor(value) && value.isRawInstance && 'client' in value; }
See knex/lib/raw.js:L131-L132
Note that these changes rely on the internals of knex (i.e. not part of the public API), and therefore could be subject to change. -
Another possibility makes changes to knex as well as objection.js such that each instance of these classes has a property which identifies it's type. For example, the
Rawknex class already has a flag (isRawInstance) that is used internally within knex and could be relied on to identify instances of each class (see above). Similarly, anisBuilderInstanceflag could be added toBuilderand anisJoinClauseInstanceflag could be added toJoinClause. Then the appropriate modifications (similar to those above) could be made within objection.js. This change would essentially be a request to make these flags part of the official public API of knex, rather than exporting the internal classes.
-