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

Feature Request: Some way to "default" global_defs that _aren't_ specified explicitly #653

Open
shicks opened this issue Apr 17, 2020 · 10 comments

Comments

@shicks
Copy link

shicks commented Apr 17, 2020

Bug report or Feature request?
Feature request.

(Apologies for all the issues, and thanks for looking into them. I've previously opened #640 and #584/#589 to attempt to address this feature request indirectly, but even if these issues were resolved, it still doesn't provide a satisfying resolution to the underlying problem.)

The basic idea is that I'm looking for a "cross-optimizer" way to provide in-source defaults for compile-time switches (i.e. variables used for conditional compilation) that aren't specified in the terser config. Closure Compiler has a more structured framework for specifying compile-time defines that allows this, and there are a number of libraries that use it to great effect. Essentially, you can write

/** @define {number} */
const BROWSER_YEAR = 2012;
/** @define {boolean} */
const ASSUME_FOO = BROWSER_YEAR > 2018;
if (!ASSUME_FOO) { /* some expensive polyfill for foo */ }

and then the conditional compilation can be controlled by either setting -d BROWSER_YEAR=2019 or -d ASSUME_FOO=true.

In another use case, one might set const DEBUG = false; const ENABLE_ASSERTS = DEBUG; const ENABLE_LOGGING = DEBUG; to have asserts and logging on by default when debugging, but allow (say) enabling only one or the other in production builds if desired.

(As an aside, Closure Library adds an additional mechanism to allow for runtime bootstrapping of uncompiled code by passing the default to goog.define(). I'm still looking to support this, but it's less immediately relevant here and just complicates things.)

The two approaches I've investigated are

  1. Use properties on process.env and substitute process.env={} as a "catch-all". This requires Add unsafe optimization to inline undefined for missing objlit props #589 to work, but even so, it's somewhat unsatisfying, and any reasonable provision for runtime bootstrapping (i.e. importing process from a module to ensure it's actually defined and prevent ReferenceErrors when uncompiled) throws further complications into it.
  2. Define a function define(name: string) that (when optimizing) just returns undefined, and then add replacements for (e.g.) define('DEBUG'). But only rollup is able to make this replacement; terser's global_defs and webpack's DefinePlugin are both too smart.

In both cases, I tried adopting a pattern of const ENABLE_ASSERTS = process.env.ENABLE_ASSERTS ?? DEBUG or define('ENABLE_ASSERTS') ?? DEBUG, which led to #640.

Rather than these complex approaches, it might be better to do something much dumber: the underlying stumbling block is that global_defs is too smart - it won't replace values that are defined in scope. If it could just be overridden (potentially in a more limited "opt-in" sort of way) then that could potentially solve this issue (though there's a ton of potential gotchas with module imports and how webpack or rollup will rewrite them, particularly across chunk boundaries - I could imagine terser not being able to intuit that some use of DEBUG actually corresponded to an opt-in definition in a different chunk).

Ignoring the module/chunk problem for now, maybe some sort of syntax like

/** @define {boolean} */
/*@__DEFINE__*/
const DEBUG = false;
/** @define {boolean} */
/*@__DEFINE__*/
const ENABLE_ASSERTS = DEBUG;

could satisfy both terser and closure at the same time?

@fabiosantoscode
Copy link
Collaborator

I sympathize with your issue. We should be able to define a lot of things at compile-time to help us get rid of code. I believe the best measure would be to carry information (Such as, is this a constant? Is this a pure function?) across module boundaries, but that would require tighter integration with the module bundler and for the bundler to have this kind of knowledge in the first place. Makes me jealous of closure compiler :) but I'm working on a Terser-integrated bundler to get around this very limitation.

But you can access globals which might not be there using typeof to check first. So for example this could also be done:

const DEBUG = typeof _DEBUG !== 'undefined' && _DEBUG     
                                                          
if (DEBUG) {                                              
    Object.assign = function () { fancyStuff() }          
    Object.assign = function () { fancyStuff() }          
}                                                         

I tried setting --define "_DEFS={foo: 'bar', debug: true}" and then using the whole obect, but I was very displeased with the results. Seems nobody tried to do this before, or they didn't open issues, because Terser happily duplicates the object and then becomes confused because there are two of them. Very cute, but not useful.

But I do think google closure compiler is onto something there. We want our definitions to work in development, we want them to be different in production, but can we compute them in realtime?

For example:

import { importedSymbol } from 'pure-module'
/*#__DEFINE__*/
const dev = process.env.NODE_ENV !== 'production'

if (dev) {
  // if we define "dev" to be true, these modules wouldn't even get imported.
  importedSymbol()
  import('./someDevThing').then(something)
}

Which when --define dev=false becomes:

// yeah, nothing

But, projects are made of many modules, so what if Terser patched a module for you? This wouldn't require an annotation to work at all.

terser file.js --define "module('./src/foo/config.js', 'dev', true)"

Where src/foo/foo.js would be:

import { dev } from "./config.js"

if (dev) {
  superExpensiveStuff()
}

And config.js could contain any exports you'd like, it's just that when Terser encounters an import of that file, it "defines" dev for you. You could even patch your process.env defining module

terser file.js --define "module('process-env-from-npm', 'default', {NODE_ENV:'production'})"

@shicks
Copy link
Author

shicks commented Apr 20, 2020

I've considered the typeof angle and am not particularly happy with it, since (1) I'd prefer to avoid making everything top-level globals, (2) I'm looking for a pattern with a little less boilerplate. Ideally there's almost no boilerplate to making a defaulted definition - the point of the default is that they're dirt-cheap to make (i.e. the fact that you can roll it out without touching downstream users is a game changer). Using something like process.env would turn this into (typeof process === 'object' && process.env && process.env.FOO) ?? false, which is pretty ugly in what should be human-readable source, and is also a lot harder to configure replacements since you need to make sure the intermediate checks all pass. (There are other permutations as well that aren't at top of mind right now, but I haven't found any I'm happy with).

The module-patching idea is an interesting one. It sounds like you're suggesting that something along those lines could provide a solution to the cross-chunk inlining issue? It's certainly worth more consideration, though my first thought is that it would be pretty difficult for Terser to plug in here, since any bundler that's used alongside would rewrite it first. So I guess it only works if Terser does the bundling (as you alluded earlier).

@fabiosantoscode
Copy link
Collaborator

Module patching could be hard to implement, as long as the bundler "gets to" the import before Terser does, which sadly is the case for Webpack. Rollup does let Terser see and change the imports because it works at the module level.

The best option is probably what you suggested with /*#__DEFINE__*/. Patching a module seems to me to be very, very tricky to achieve, as module bundlers have different concerns than Terser and the one I'm building might turn out to be terrible, or never get any adoption.

@justinfagnani
Copy link

My team hit this issue just now - we only have modules in our project and don't define any global variables. We'd like to use global_defs with top-level module variables, specifically to replace const DEV_MODE = true; with const DEV_MODE = false;.

@fabiosantoscode
Copy link
Collaborator

If you use webpack for development and production as well, webpack's DefinePlugin can be a good alternative.

You would use DEV_MODE without defining it anywhere, and have it replaced.

It's not really a global because it won't end up in the window.

But it's not possible (as far as I know) to scope this change to only your modules, so you either need some prefix or some sort of DefineLoader (because loaders apply to each file and you can exclude node_modules from this replacement)

@justinfagnani
Copy link

We don't use bundlers during development though.

@fabiosantoscode
Copy link
Collaborator

Yeah, this is the main problem. You'd have to set an actual global during development, maybe in your entry file you can do this:

global.MyIsDev = true

Since Terser is not your bundler for development it becomes harder to avoid this.

@justinfagnani
Copy link

I don't understand why though. If Terser can replace text, why can't it replace:

const DEV_MODE = true;

with

const DEV_MODE = false;

That the declaration is in a module doesn't seem like it should make it impossible.

@fabiosantoscode
Copy link
Collaborator

In theory, it's possible. But Terser isn't the bundler, so it doesn't know where that constant might be exported to.

And it doesn't track the flow of variables (besides superficial moves visible from just one closure) so when you export from one module to the next, the information of what variable name it is or where it came from is lost in translation.

@fabiosantoscode
Copy link
Collaborator

I've considered a number of possible solutions to this issue, like for instance adding an option to have the bundler communicate to Terser the connections between variables, or magic comments to be placed when the bundler exchanges import { DEV_MODE } from '...' with its own complex machinery.

But none of the options I've considered seems especially feasible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants