Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide the ability to remove code needed only in dev mode from the production bundle #51175

Open
markostanimirovic opened this issue Jul 26, 2023 · 19 comments
Labels
area: core Issues related to the framework runtime feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature
Milestone

Comments

@markostanimirovic
Copy link
Contributor

Which @angular/* package(s) are relevant/related to the feature request?

core

Description

Several NgRx libraries have features/checks that are only used in development mode (e.g. store runtime checks, component store lifecycle hooks check, store devtools, etc).

Proposed solution

Expose the ngDevMode/isNgDevMode variable to the public API, so it can be used in Angular libraries that have dev mode-related functionalities, but also in user codebases if needed.

This feature could significantly reduce the size of production bundles in many Angular codebases.

Alternatives considered

Currently, we can use ngDevMode to remove dev mode-related logic from the production bundle in the following way:

declare const ngDevMode: boolean;
const isNgDevMode = typeof ngDevMode === 'undefined' || !!ngDevMode;

// ...

if (isNgDevMode) {
  console.warn('@ngrx/store: ...');
}

However, this global variable is not part of the public API and it can be risky to rely on it.

@JeanMeche
Copy link
Member

JeanMeche commented Jul 26, 2023

Hi Marko, we already have isDevMode, have you tried it ?

@markostanimirovic
Copy link
Contributor Author

Yes, we're using the isDevMode function, but we would like to eliminate "dead" code from the production bundle which is possible with ngDevMode.

@JeanMeche
Copy link
Member

JeanMeche commented Jul 26, 2023

Well isDevMode is ngDevMode under the hood.

export function isDevMode(): boolean {
return typeof ngDevMode === 'undefined' || !!ngDevMode;
}

And terser should be able to strip the code that is behind if (isDevMode()) {}

@arturovt
Copy link
Contributor

arturovt commented Jul 26, 2023

Terser is not able to tree-shake the code if the function, returning a statically analyzeble value, is used in more than 1 module. Terser is not analyzing the entire project's dependency graph, that's why ngDevMode is defined everywhere in the Angular code (and not isDevMode()), which means it doesn't perform cross-module tree-shaking. You can check the following example:

// file-1.js
import { isDevMode } from './shared';

isDevMode() && console.log('file-1');

// file-2.js
import { isDevMode } from './shared';

isDevMode() && console.log('file-2');

// shared.js
export function isDevMode() {
  return !!ngDevMode;
}
// webpack.config.js
const TerserPlugin = require('terser-webpack-plugin');

module.exports = {
  entry: ['./src/file-1.js', './src/file-2.js'],
  optimization: {
    minimize: true,
    minimizer: [
      new TerserPlugin({
        terserOptions: {
          compress: {
            global_defs: {
              ngDevMode: false,
            },
          },
        },
      }),
    ],
  },
};

The output code:

(() => {
  'use strict';
  var e = {
      768: (e, r, o) => {
        function t() {
          return !1;
        }
        o.d(r, { X: () => t });
      },
    },
    r = {};
  function o(t) {
    var n = r[t];
    if (void 0 !== n) return n.exports;
    var i = (r[t] = { exports: {} });
    return e[t](i, i.exports, o), i.exports;
  }
  (o.d = (e, r) => {
    for (var t in r)
      o.o(r, t) &&
        !o.o(e, t) &&
        Object.defineProperty(e, t, { enumerable: !0, get: r[t] });
  }),
    (o.o = (e, r) => Object.prototype.hasOwnProperty.call(e, r)),
    (0, o(768).X)() && console.log('file-1'),
    (0, o(768).X)() && console.log('file-2');
})();

If we update the entry property to include only ./src/file-1.js, the output code becomes the following:

(()=>{"use strict"})();

@markwhitfeld
Copy link
Contributor

So, to understand this correctly, in order to achieve what is needed, would angular need to:

  1. export the ngDevMode as a variable,
  2. or add it to the global typescript namespace (in the angular lib)

to make this available to libs to be useful for tree shaking?

@AndrewKushnir AndrewKushnir added feature Issue that requests a new feature area: core Issues related to the framework runtime labels Jul 26, 2023
@ngbot ngbot bot added this to the Backlog milestone Jul 26, 2023
@JeanMeche
Copy link
Member

  1. export the ngDevMode as a variable,

ngDevMode is not a conventional constant. It's set the by the cli and declared as global inside the framework.

declare global {
/**
* Values of ngDevMode
* Depending on the current state of the application, ngDevMode may have one of several values.
*
* For convenience, the “truthy” value which enables dev mode is also an object which contains
* Angular’s performance counters. This is not necessary, but cuts down on boilerplate for the
* perf counters.
*
* ngDevMode may also be set to false. This can happen in one of a few ways:
* - The user explicitly sets `window.ngDevMode = false` somewhere in their app.
* - The user calls `enableProdMode()`.
* - The URL contains a `ngDevMode=false` text.
* Finally, ngDevMode may not have been defined at all.
*/
const ngDevMode: null|NgDevModePerfCounters;

@angular-robot angular-robot bot added the feature: votes required Feature request which is currently still in the voting phase label Jul 27, 2023
@angular-robot
Copy link
Contributor

angular-robot bot commented Jul 27, 2023

This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.

You can find more details about the feature request process in our documentation.

@maxime1992
Copy link

I've been using a little trick in my case with ngrx which is to have 2 files in which I define what I want, for example the redux dev tool import. In dev mode I import it, while in the other file I don't. I then use the angular cli swapping file capability to use the dev one at serve time and the prod one (with nothing it in except an empty array) at build time.

And because I feel like I'm absolutely not clear (end of day sorry 🙃) here's what I've got:

ngrx.ts:

import { ModuleWithProviders } from '@angular/core';
import { StoreDevtoolsModule } from '@ngrx/store-devtools';

// this file will be replaced by the prod version at build time!
// this is to make sure that we don't include `store-devtools` in our prod bundle
// https://ngrx.io/guide/store-devtools/recipes/exclude

export const ngrxModules: ModuleWithProviders<any>[] = [StoreDevtoolsModule.instrument({ maxAge: 25 })];

ngrx.prod.ts:

import { ModuleWithProviders } from '@angular/core';

export const ngrxModules: ModuleWithProviders<any>[] = [];

And in my Angular CLI config file:

          "fileReplacements": [
            {
              "replace": "apps/my-app/src/app/build-specifics/ngrx.ts",
              "with": "apps/my-app/src/app/build-specifics/ngrx.prod.ts"
            }
          ],

Now, while there's a work around for this... I have to admit that this feature would be a really nice to have. It's quite convoluted and error prone to swap files at build time. Meanwhile, in case anyone needs a workaround to make sure you don't bundle the entire code for the redux dev tool or others, you know where to look :)

@simeyla
Copy link

simeyla commented Jul 27, 2023

Edit: I've deleted most of my answer due to making an incorrect assumption about optimized constants. Unfortunately it looks like terser isn't smart enough to optimize constants imported from environment.ts (related to the use of webpack require).


I wouldn't ever want to use ngDevMode in application code - but for libraries perhaps it makes more sense.

@JeanMeche
Copy link
Member

JeanMeche commented Jul 27, 2023

@simeyla The usecase forngDevMode is definitely oriented toward libraries that want to provide a similar behaviour as Angular :

Explicit logs and errors in devMode & only critical errors in prodMode (and less verbose).

@yharaskrik
Copy link
Contributor

@simeyla The usecase forngDevMode is definitely oriented toward libraries that want to provide a similar behaviour as Angular :

Explicit logs and errors in devMode & only critical errors in prodMode (and less verbose).

This. Please. I have been wanting to do this for a while! Will help us library authors provide really clear and complete verbose logging but then have prod bundles be as small as possible.

@alan-agius4
Copy link
Contributor

Note: ngDevMode is considered as a private API and is not intended but to be used by 3rd parties.

@JeanMeche
Copy link
Member

Yes, that's stated by @markostanimirovic in his post. The request here would be to make it public so library maintainers can officially rely on it.

@angular-robot angular-robot bot added feature: under consideration Feature request for which voting has completed and the request is now under consideration and removed feature: votes required Feature request which is currently still in the voting phase labels Jul 28, 2023
@JeanMeche
Copy link
Member

Shouldn't we close this issue in favor of the one on the CLI repo ? angular/angular-cli#23738

@simeyla
Copy link

simeyla commented Apr 12, 2024

@JeanMeche If so it needs a new title. The title of this question is much more suitable and easy to find.

I did want to add an alternative approach (not sure whether or not this works for library authors since I've never created such a library).

In angular.json you can now add constants under 'define'. This is similar to the [webpack DefinePlugin] and esbuild equivalent (https://webpack.js.org/plugins/define-plugin/). However since it's now in angular.json it will work with both build systems.

I couldn't immediately find official mention of options.define in the angular.json docs, but here's a article https://netbasal.com/explore-angular-clis-define-option-effortless-global-identifier-replacement-f08fec7d9243

@alan-agius4
Copy link
Contributor

@JeanMeche, actually the CLI issue is rather working as expected, the missing part is having a public API similar to ngDevMode that is treeshakable.

Worth to mention that the story around ngDevMode also changed over the years as originally this was not meant to be used to as a token to remove code not even in the framework code case, but rather more for dev mode runtime stats.

options.define can definitely be used as a valid alternative. @markostanimirovic, mind opening a new issue in this repo for a request to document the define option. Likely it should be included in https://angular.dev/reference/configs/workspace-config#complex-configuration-value

@k3nsei
Copy link

k3nsei commented Apr 13, 2024

@alan-agius4 @simeyla But how would options.define work for library authors? While we are building libraries we want to include this code when publishing to NPM. We only want to remove this piece of code when building an application that uses the library. Angular is opinionated and this token should also be common for the whole community. To not introduce 3, 5 or 10 different tokens with options.define as every library author would choose different name.

So I personally don't see making changes to angular.json as a solution here. We need something built into the framework itself. That would be commonly used and not a source of fragmentation. It's always possibility to introduce more async capabilities to framework and allow provider factories to use Promises then we can just use dynamic imports and just based on condition load different package in runtime. Having APP_INITIALIZER isn't help here as they conditionally blocking startup, as it's well know issue that it isn't working with router guards and resolvers when initial navigation is enabled.

@jkrems
Copy link
Contributor

jkrems commented Sep 30, 2024

Would it be reasonable to support import.meta.env.DEV? It looks like that has some buy-in from a couple of bundlers (rspack, vite, bun to some extend).

@JeanMeche
Copy link
Member

JeanMeche commented Sep 30, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature
Projects
None yet
Development

No branches or pull requests