simondotm / nx-firebase

Firebase plugin for Nx Monorepos
https://www.npmjs.com/package/@simondotm/nx-firebase
MIT License
175 stars 31 forks source link

esbuild is bundling external dependencies #177

Closed pfpoujol closed 3 months ago

pfpoujol commented 6 months ago

nx-firebase v2.1.2 has problemes with Nx v17. Building functions creates a gigantic main.js file. And the deployment command fails. Works fine with Nx v16.

main.js sample (191524 lines)

var __create = Object.create;
var __defProp = Object.defineProperty;
var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
var __getOwnPropNames = Object.getOwnPropertyNames;
var __getProtoOf = Object.getPrototypeOf;
var __hasOwnProp = Object.prototype.hasOwnProperty;
var __require = /* @__PURE__ */ ((x) => typeof require !== "undefined" ? require : typeof Proxy !== "undefined" ? new Proxy(x, {
  get: (a, b) => (typeof require !== "undefined" ? require : a)[b]
}) : x)(function(x) {
  if (typeof require !== "undefined")
    return require.apply(this, arguments);
  throw Error('Dynamic require of "' + x + '" is not supported');
});
var __esm = (fn, res) => function __init() {
  return fn && (res = (0, fn[__getOwnPropNames(fn)[0]])(fn = 0)), res;
};
var __commonJS = (cb, mod4) => function __require2() {
  return mod4 || (0, cb[__getOwnPropNames(cb)[0]])((mod4 = { exports: {} }).exports, mod4), mod4.exports;
};
var __export = (target, all) => {
  for (var name in all)
    __defProp(target, name, { get: all[name], enumerable: true });
};
var __copyProps = (to, from, except, desc) => {
  if (from && typeof from === "object" || typeof from === "function") {
    for (let key of __getOwnPropNames(from))
      if (!__hasOwnProp.call(to, key) && key !== except)
        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
  }
  return to;
};
var __toESM = (mod4, isNodeMode, target) => (target = mod4 != null ? __create(__getProtoOf(mod4)) : {}, __copyProps(
  // If the importer is in node compatibility mode or this is not an ESM
  // file that has been converted to a CommonJS file using a Babel-
  // compatible transform (i.e. "__esModule" has not been set), then set
  // "default" to the CommonJS "module.exports" for node compatibility.
  isNodeMode || !mod4 || !mod4.__esModule ? __defProp(target, "default", { value: mod4, enumerable: true }) : target,
  mod4
));
var __toCommonJS = (mod4) => __copyProps(__defProp({}, "__esModule", { value: true }), mod4);

my main.ts

/**
 * Import function triggers from their respective submodules:
 *
 * import {onCall} from "firebase-functions/v2/https";
 * import {onDocumentWritten} from "firebase-functions/v2/firestore";
 *
 * See a full list of supported triggers at https://firebase.google.com/docs/functions
 */

import {onRequest} from 'firebase-functions/v2/https';
import * as logger from 'firebase-functions/logger';
import {setGlobalOptions} from "firebase-functions/v2";
import {getAuth, UpdateRequest} from "firebase-admin/auth";

initializeApp();
setGlobalOptions({ region: 'europe-west9' });

// Start writing functions
// https://firebase.google.com/docs/functions/typescript

 export const updateAuthUser  = onRequest(async (req, res) => {
   try {
     const body = req.body as {uid: string, data: UpdateRequest};
     const userRecord = await getAuth().updateUser(body.uid, body.data);
     res.status(200).send(userRecord);
   } catch (e) {
     logger.error(e);
     res.status(500).send(e);
   }
 });
simondotm commented 6 months ago

Blerg - this is esbuild bundling external dependencies. It shouldn't do this, but I've see it do it sometimes with some Nx versions.

Please can you share which Nx version you were using?

I'll try and look at this soon.

pfpoujol commented 6 months ago

Hello, it was nx v17.1.3

simondotm commented 6 months ago

Quick update on this: @nx/esbuild executor option thirdParty=false tells Nx esbuild to NOT include external dependencies in the bundle.

I've got a compatibility test suite here that generates an Nx workspace for every Nx version, installs the plugin and generates/builds a firebase + function app project to verify compatibility.

Guess what, it passes in Nx version 17.3.1 (the version you are having problems with) but generates the faulty mega-bundle in versions 16.1.4, 16.2.2, 16.3.2, 16.4.3, 16.5.5, 16.6.0, 16.7.4. Nx version 16.8.1 onwards is fine.

This is highly suspect, because it didn't do this for me before. Some fragile code somewhere in Nx is my hunch right now.

I will keep digging a bit now I have a repro.

simondotm commented 6 months ago

Ran the same test suite again in a different output directory and now all of the output bundles are correct 🤔

simondotm commented 6 months ago

Another update, I went back to the original test suite folder and ran npx nx reset followed by npx nx build functions manually (rather than from my automated test suite) and this time that also builds the correct bundle.

@pfpoujol if you get a chance, please could you try the same cmd line sequence:

  1. npx nx reset
  2. npx nx build functions (or whatever your function app project above is called)

And see if the bundle still contains external dependencies?

I dont see why the behaviour would change since the esbuild options are the same, but trying to home in on the problem here.

pfpoujol commented 6 months ago

Hi @simondotm sorry for the late reply. I tried the same cmd line sequence and the bundle still contains external dependencies.

simondotm commented 6 months ago

The problem seems to lie here in the Nx esbuild plugin: https://github.com/nrwl/nx/blob/f1849a792d477c287841900bf1657fb73d6ad80b/packages/esbuild/src/executors/esbuild/esbuild.impl.ts#L64

In my local setup where esbuild incorrectly bundles external deps even when thirdParty is false, Nx does not seem to be detecting there are external dependencies at all (thirdPartyDependencies array is empty) so it goes ahead and bundles everything.

I dont know yet why this happens. :/

thekip commented 5 months ago

As a workaround i use my custom esbuild config and set packages: 'external' which since esbuild@0.19 correctly understand local aliased packages and packages from node_modules. You can pass this configuration either in the esbuildOptions: {} in project.json or in the separate config "esbuildConfig": "{projectRoot}/build.cjs", which i choosed because wanted to use esbuild plugins:

const { swcPlugin } = require('esbuild-plugin-swc');

/** @type {import('esbuild').BuildOptions}  */
module.exports = {
  plugins: [
    swcPlugin({
      jsc: {
        parser: {
          syntax: 'typescript',
          decorators: true,
        },
        transform: {
          decoratorMetadata: true,
          legacyDecorator: true,
        },

        externalHelpers: true,
        target: 'esnext',
      },
    }),
  ],
  treeShaking: true,
  packages: 'external',

  tsconfig: '../../tsconfig.base.json',
  logLevel: 'info',
};
mackelito commented 5 months ago

Any updates here? I'm seeing the same errors nx reset did not work for me either..

simondotm commented 3 months ago

I'm starting to wonder if the plugin should generate an esbuild config for firebase functions in the workspace root, which all function projects use. This way, it is easier to customise function builders for plugins, and control behaviour externally to Nx.

simondotm commented 3 months ago

Another possibility is that issue #191 is causing problems by pulling in incorrect @nx/esbuild plugin versions for the workspace nx version.

I've been running & tweaking my compatibility tests the past two days, and versions since Nx 16.8.1 are reliably passing the esbuild mega bundle check, whereas prior versions are unreliable on this test.

I'm going to fix 191 and then see if it resolves this issue too.

simondotm commented 3 months ago

I'm reasonably confident that the changes in plugin release v0.2.2 combined with use of Nx versions >=16.8.1 no longer manifests this issue.

The next plugin release will require a minimum Nx version of 16.8.1 to help ensure this.

Closing for now.