Provide entry point for umzug 'core' for better browser usage? [v3]

[patdx]

• Version: v3.0.0-beta.16
• Storage: Custom wrapper for sql.js in the browser and @capacitor-community/sqlite
• Node version: 14.17.3
• Dev setup: Mobile iOS/Android app using Next.js, Ionic and Capacitor

Hello, I'm trying out the v3 beta for developing a mobile app, it's quite nice and feels lightweight . :)

I might be wrong but I got the impression from the changelog for v3 and the example here: https://github.com/sequelize/umzug/tree/master/examples/7.bundling-codegen that umzug v3 is going in a more platform-generic direction, so able to be used with webpack and browser targets easily.

Functionally, it does seem to work with webpack and browser already, however, I get the following webpack warnings on every recompile of my app:

warn  - ../../node_modules/@rushstack/ts-command-line/node_modules/colors/lib/extendStringPrototype.js
Critical dependency: the request of a dependency is an expression

../../node_modules/umzug/lib/umzug.js
Critical dependency: the request of a dependency is an expression

./node_modules/umzug/lib/umzug.js
Critical dependency: the request of a dependency is an expression

./node_modules/colors/lib/extendStringPrototype.js
Critical dependency: the request of a dependency is an expression

I am using Next.js, but to keep things simple, I created a super minimal reproduction example using create-react-app here: https://github.com/patdx/umzug-browser-demo. Just yarn and yarn start to see the warnings.

This kind of warning indicates a deoptimization where a lot of extra code may get loaded into the browser:

Webpack tries to resolve require calls statically to make a minimal bundle. When a library uses variables or expressions in a require call (such as require('' + 'nodent') in these lines of ajv), Webpack cannot resolve them statically and imports the entire package. source

For browser usage, it would be nice if there was a way to import just the Umzug core without any code that triggers these webpack warnings.

I tried importing directly from the umzug file instead:

import { Umzug } from "umzug/lib/umzug";

However this had no change, I think due to 2 parts:

• defaultResolver
• getCli and runAsCli

The parts seem specific to Node.js and not essentially for umzug's core operation. I wonder if it is possible to shift these two parts into a separate file? Here is one idea how to do it:

// umzug-core.ts

export class UmzugCore {
  // everything from Umzug.ts 
}

// umzug.ts

export class Umzug extends UmzugCore {
  constructor(options: UmzugOptions<Ctx>) {
    // just a demo, I guess the API needs to be tweaked a little to preserve `readonly`
    if (!options.migration.resolve) {
      options.migration.resolve = Umzug.defaultResolver;
    }

    super(options);
  }

  static defaultResolver = { /* ... */ }

  protected getCli(options?: CommandLineParserOptions): UmzugCLI {
    return new UmzugCLI(this, options);
  }

  async runAsCLI(argv?: string[]): Promise<boolean> {
    const cli = this.getCli();
    return cli.execute(argv);
  }
}

(Note: It may be nice to do something similar with the default JSON storage adapter, which also depends on fs, but this doesn't seem to trigger an warning w/ create-react-app or Next.js.).

With this setup, I think the most common user, Node.js user, could continue to use the default import { Umzug } from 'umzug' with easy defaults for Node.js, and the more rare browser user could use something like import { UmzugCore } from 'umzug/lib/umzug-core'.

[mmkal]

This is a pretty fair request, to separate out node-specific parts of umzug - but I kind of object to needing to do it. If webpack could be configured to just load nothing instead of everything, and leave the responsibility on the user, then this would be a non-issue. Is that possible? I believe I've handled similar problems with something like:

  plugins: [
    new webpack.ContextReplacementPlugin(/[/\\]umzug[/\\]/, data => {
      data.dependencies.forEach(d => delete d.critical)
      return data
    }),
  ],

Although that's fairly hacky and nothing about it guarantees that you won't accidentally include a ton of useless files. Are there any other workarounds on the webpack side you're aware of?

Another consideration is that if this was done and made an official part of the API surface, we'd be tied to the typescript compiled output folder structure, which isn't great. It might require looking into a tool like microbundle - which is something I've been meaning to do anyway, but I can't promise it'll get done any time soon.

[patdx]

That makes sense, thanks for the consideration.

Are there any other workarounds on the webpack side you're aware of?

Hmm I think I’ve done similar things with

https://webpack.js.org/loaders/null-loader/
https://www.npmjs.com/package/string-replace-loader

so they may make good options too?

String replace loader has a “strict” mode so it can be set to error out if the replacement doesn’t work.

will try it out shortly and try to report back…I’m working on a different project at the moment but expect to resume on this project in 1~2 weeks. Feel free to close this issue at your convenience. Thanks!