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

Namespaced class not working #1319

Closed
moimael opened this issue Jun 5, 2018 · 5 comments
Closed

Namespaced class not working #1319

moimael opened this issue Jun 5, 2018 · 5 comments

Comments

@moimael
Copy link

moimael commented Jun 5, 2018

decaffeinate is producing the wrong JavaScript based on my CoffeeScript input:

class IZ.Controllers.Page extends IZ.Controller

I get this output:

const Cls = (IZ.Controllers.Page = class Page extends IZ.Controller {

Not sure how the javascript should look like but I get this error:

ReferenceError: invalid assignment left-hand side
@alangpierce
Copy link
Member

Hi @moimael, thanks for the report. Could you give a more complete code example showing the error, ideally as a repl link?

If I just run decaffeinate on your code snippet, it gives this output:

IZ.Controllers.Page = class Page extends IZ.Controller {};

Repl link

so I think you'll need a more complete example. Ideally there's a way to trim your code down to a small self-contained test case demonstrating the issue.

From the error, it looks to me like maybe IZ.Controllers isn't defined. Keep in mind that class IZ.Controllers.Page extends IZ.Controller is a special CoffeeScript syntax that assigns the new class to IZ.Controllers.Page, so IZ.Controllers needs to already exist. That syntax doesn't exist in JavaScript; classes always are expressions or are assigned to a new normal variable, not an object property.

Also, would be good to double-check that this case works in the CoffeeScript compiler and doesn't work in decaffeinate.

@moimael
Copy link
Author

moimael commented Jun 5, 2018

Hi,
Thanks for your swift answer :)

Here you go:

REPL

The thing is it is used in a rails project so the imports are handled via the rails asset system. IZ.Controllers is defined but in another file. It does work with coffeescript compiler, this is actual (old) production code.

@moimael
Copy link
Author

moimael commented Jun 19, 2018

@alangpierce Anything else I can do to help solve this issue ?

@eventualbuddha
Copy link
Collaborator

@moimael I believe this may actually be correct. Your example code in the REPL does not have the same error you initially mentioned (ReferenceError: invalid assignment left-hand side), but is instead IZ is not defined. That makes perfect sense as nothing defines IZ in your example.

However, if there really is a problem with decaffeinate that leads to the error you mentioned (ReferenceError: invalid assignment left-hand side), we don't currently have enough information to debug it. Can you produce a REPL link that throws with that error?

@alangpierce
Copy link
Member

Sorry for the radio silence!

I just played around with it a bit more. ReferenceError: invalid assignment left-hand side is Firefox's phrasing for assignments that are completely invalid, like 3 = 4;. That's different from assignments like IZ.Controllers.Page = class Page {} that just don't resolve correctly.

@moimael It looks like you have a subclass with bound methods, so the Babel/TypeScript workaround code is being generated (see the comment // Hack: trick Babel/TypeScript into allowing this before super.), and my best guess is that you're not compiling it with Babel/TypeScript (targeting ES5). If you don't use Babel or TypeScript, the code ends up running this = this;, which will crash with the error you gave.

In short, you must either set up your build system to use Babel (which you need to do anyway if you want to support IE) or rewrite the code to bind the methods after the super call in the constructor (which changes the behavior, so you might need to rework the code more to fix any resulting bugs). I'd strongly recommend reworking your code, either on the CoffeeScript side to get rid of => methods or on the JavaScript side by rearranging the constructor. Unfortunately, this is the main thing that decaffeinate can't do automatically, and the hack is just there since it helps ease the transition in some cases.

I wrote up some docs about it here: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md#ds001-remove-babeltypescript-constructor-workaround

If that's not the issue, would be good to get more information.

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