Skip to content

Exception thrown when objection.js is minified #2123

@mattharcourt

Description

@mattharcourt

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
  1. 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.

  2. The second solution is to partially minify the code by utilizing the more granular --minify-whitespace and --minify-syntax options of esbuild instead of the more general --minify option (i.e. avoiding identifier minification which can be seperately controlled via the --minify-identifiers option). This will avoid the issue while still providing some reduction in the size of the bundle (though less reduction than with full minification).

  3. 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, and isKnexRaw found in knexUtils.js. Here are three potential modifications that could be made:

    1. The obvious solution is to utilize instanceof to 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 the Builder, JoinClause and Raw classes are exported publicly, so that the appropriate identifier is used even when minified. Then the objection.js knexUtils.js checks 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

    2. The next possibility restricts changes to within the objection.js file
      knexUtils.js where the isKnexQueryBuilder, isKnexJoinBuilder, and isKnexRaw functions 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.

    3. 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 Raw knex 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, an isBuilderInstance flag could be added to Builder and an isJoinClauseInstance flag could be added to JoinClause. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions