Skip to content

Decorators cause unwanted warnings about = vs == when code is run in gjs #31774

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

Closed
realh opened this issue Jun 5, 2019 · 5 comments
Closed
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@realh
Copy link

realh commented Jun 5, 2019

TypeScript Version: 3.6.0-dev.20190604

Search Terms:
test for equality (==) mistyped as assignment (=)?
test for equality (==) mistyped as assignment (=)? decorator

Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.
// Requires --experimentalDecorators=true

function deco(klass: Function) {
}

@deco
class TestClass {
}

Expected behavior:
When the transpiled script is run it should not print any warnings.

Actual behavior:
When run in gjs (which is based on Mozilla Spidermonkey) it causes a warning like this:
Gjs-Message: 13:59:43.346: JS WARNING: [index.js 4]: test for equality (==) mistyped as assignment (=)?

It's because the generated code includes this if clause: if (d = decorators[i]). I don't think this is an error, but I think it would be better to write it as if ((d = decorators[i]) != null) to make its intention clear and prevent false alarms which end users won't want to see.

Playground Link:

Related Issues:
No, but it would probably be a good idea to test at least other types of decorator to check that they don't independently cause the same issue.

@fatcerberus
Copy link

Hmm, while I see your point, the question with this kind of thing is, where do you draw the line? What you’re essentially seeing is a lint warning, and TypeScript can’t possibly satisfy every linter and lint configuration in the world. At the end of the day, tsc is a compiler; as long as the code it produces isn’t misbehaving at runtime, you shouldn’t need to care what the generated code looks like.

That being said, it’s unfortunate that in this particular case, some kind of linting is being done at runtime, which IMO is pretty bizarre, especially in this day and age of transpilers and minification everywhere.

@nattthebear
Copy link

Linting generated code seems doomed to failure. But, the TS team has accepted such changes before: #27312

@fatcerberus
Copy link

fatcerberus commented Jun 5, 2019

@nattthebear Yeah, the thing here is, there’s some runtime (this “gjs”) which automatically lints the code it runs - so it’s a different case than a developer choosing to lint the generated code. The user actually has no choice in the matter (which is misguided on gjs part but what can you do).

Alas, the complexities inherent in JS being both “the assembly language of the web” and “source code” simultaneously...

@realh
Copy link
Author

realh commented Jun 6, 2019

gjs provides interoperability between SpiderMonkey and gobject-introspection, so you can write applications in JS using libraries like GTK and GStreamer. The warnings come from SpiderMonkey, so I suspect they would also appear in Firefox's developer console if TS decorators were used in a web app, but I haven't verified that.

I turns out gjs does have a way I can disable these warnings so I don't mind personally if you want to close this, but I'll leave the final decision up to you.

@RyanCavanaugh RyanCavanaugh added the External Relates to another program, environment, or user action which we cannot control. label Jun 13, 2019
@RyanCavanaugh
Copy link
Member

The code is correctly written, and is written in a way to be as short as possible to minimize download size. A runtime that errors on a legal assignment isn't a JS runtime and we're not capable of forming our output to conform to all third-party opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

5 participants
@nattthebear @fatcerberus @RyanCavanaugh @realh and others