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

Passing @define down to closure compiler #434

Closed
rubenlg opened this issue Mar 15, 2017 · 11 comments
Closed

Passing @define down to closure compiler #434

rubenlg opened this issue Mar 15, 2017 · 11 comments

Comments

@rubenlg
Copy link
Contributor

rubenlg commented Mar 15, 2017

I suspect this might have worked at some point after reading Martin's comment here: angular/ts-minify#59

However, I tried the following two approaches today and none worked:

  1. Create a non-module ts file with an explicit annotation:
/** @define */
const DEBUG = false;

This doesn't work because tsickle generates a closure module even though this typescript file is not a module to begin with (i.e. it doesn't export anything). And closure compiler can't override @defines inside modules, as per google/closure-compiler#1601.

  1. Use a d.ts for typescript, and inject a JS file with the @define before calling closure:

.d.ts:

declare namespace debug {
  const DEBUG: boolean;
}

.js

var debug = {};
/** @export {boolean} */
debug.DEBUG = false;

This doesn't work because the externs generated by tsickle also declare both debugand debug.DEBUG, which clash with the js file when running the closure compiler.

What did work, was putting that constant in a regular .js file using goog.provide() instead of goog.module(), and then using clutz to make that available to typescript in the form of a module declaration. Besides being really cumbersome, that approach doesn't work for closure's own goog.DEBUG because closure/goog/base.js doesn't have a goog.provide.

Both seem worth fixing:

In 1 I would expect tsickle to not emit a module, since the typescript file is also not a module.

In 2, it would be useful to be able to annotate a declare with some @doNotExport annotation, if we know that something will declare that later during closure compilation (like closure/goog/base.js). It seems like tsickle already has some way of ignoring d.ts files living inside node_modules, because it doesn't generate externs for those, so perhaps that could be made controllable by the programmer?

@mprobst
Copy link
Contributor

mprobst commented Mar 16, 2017

Why don't you pass --define from the command line?

Or maybe to restate: what are you trying to do?

@rubenlg
Copy link
Contributor Author

rubenlg commented Mar 16, 2017

$ tsickle test.ts --define DEBUG=true
unknown flag '--define'

If you mean on the closure compiler, --define warns if there is no code with @define on something. And if that @define is on a goog.module, then there is no way to alter it via --define.

What I was trying to do is have some compile-time flags to change how the code behaves depending on the target environment (dev/staging vs production).

@mprobst
Copy link
Contributor

mprobst commented Mar 16, 2017 via email

@rubenlg
Copy link
Contributor Author

rubenlg commented Mar 16, 2017

Using defines already present in javascript (e.g. goog.DEBUG) also helps, but there the problem is that you need a d.ts file to make that visible to typescript, and tsickle emits an extern for it. Now the closure compiler sees the symbol both in the javascript defining it and the externs, which is an error.

@mprobst
Copy link
Contributor

mprobst commented Mar 16, 2017 via email

@rubenlg
Copy link
Contributor Author

rubenlg commented Mar 16, 2017

Good point, I haven't looked yet at how clutz handles closure/goog/base.js to make goog.DEBUG available. Ignore my last comment then.

Thanks for the detailed proposal to have @define available on typescript. It makes a lot of sense, and would finally enable compile-time flags on typescript. Beyond eliminating debug code, it also helps configuring other environmental properties like the database to use for dev/staging/prod.

@evmar
Copy link
Contributor

evmar commented Sep 17, 2019

I think this now works:

// file: my_flag.ts
/** @define */
const MY_FLAG = goog.define('my.project.MY_FLAG', false);

export function logFlag() {
  console.log(`flag is ${MY_FLAG}`);
}

@evmar evmar closed this as completed Sep 17, 2019
@evmar
Copy link
Contributor

evmar commented Sep 17, 2019

Oh wait, I confused goog.define with @define.

@evmar evmar reopened this Sep 17, 2019
@mprobst
Copy link
Contributor

mprobst commented Sep 17, 2019 via email

@rubenlg
Copy link
Contributor Author

rubenlg commented Sep 17, 2019

@mprobst what's the change from @sdh? I see that google/closure-compiler#1601 is still open.

In order for the example from @evmar to work correctly, you need proper typings of goog.define. I.e. it has to infer the type of the constant correctly. This should work:

declare namespace goog {
  function define<T>(namespace: string, defVal: T): T;
}

If you want to run your code also through tsc (or ts-node or jasmine), using goog.define means you have to depend on the closure library, because goog won't be available at runtime.

It would be awesome if tsickle was able to perform the following transformation:

// my_module.ts
/** @define */
export const MY_FLAG = false;

to

// my_module.js
/** @define {boolean} */
exports.MY_FLAG = goog.define('path.to.my_module.MY_FLAG', false);

That would enable using other processors than tsickle during development without depending on the closure library.

@mprobst
Copy link
Contributor

mprobst commented Sep 17, 2019

Ack, but I think that's far out of scope for tsickle. It's not intended to polyfill @defines or other features of Closure Library/Compiler, it's just intended to insert appropriate types and bridge the module system.

@mprobst mprobst closed this as completed Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants