cincheo / jsweet

A Java to JavaScript transpiler.
http://www.jsweet.org
Other
1.46k stars 160 forks source link

Import problems introduced in 2.3.6 release #574

Closed EricWittmann closed 4 years ago

EricWittmann commented 4 years ago

We have a data modeling library for OpenAPI and AsyncAPI located here: https://github.com/Apicurio/apicurio-data-models

We use JSweet to transpile from Java to TS so that we can release the library as both java (in maven central) and also as javascript (in npmjs.com). Our project no longer works as of the JSweet 2.3.6 release. The problem seems to be around imports. The 2.3.6 version appears to try to avoid importing some things that maybe it doesn't believe are needed. I've been comparing the transpiled typescript from 2.3.5 vs. the transpiled typescript from 2.3.6 and spot checking various classes. The differences appear to be exclusively around imports.

In some cases the imports are simply missing, and in other cases the imports are now at the bottom of the transpiled file rather than the top.

Interestingly, it seems that some of these missing imports really were not needed. However, in at least some cases the missing imports were actually needed, and the code won't run without them. Here is an example:

image

I can't find a tag or branch in the JSweet repository for either 2.3.5 or 2.3.6, or I would have diff'd the codebase to see if I could find the issue.

I'm attaching the generated/transpiled results to this issue, which I have sanitized a bit to highlight just the real differences (e.g. I did a find/replace on the "Generated by JSweet" comment on each file).

apicurio-data-models-TS.zip

Any other information needed, please let me know. Or if there is a way to diff the JSweet codebase between 2.3.6 and 2.3.5 so I can perhaps track down the issue myself I'm happy to do that.

lgrignon commented 4 years ago

This is really interesting, thank you very much @EricWittmann for reporting it!

Something that could help a lot is a PR with a minimal unit test case showing the problem and the expected behavior. Our unit tests are standard and straightforward (IMHO :)) JUnit tests. Please tell me if you need any help with this.

EricWittmann commented 4 years ago

Sure, I'll try to come up with a simple reproducer project that will perhaps be easy to turn into a JUnit test.

EricWittmann commented 4 years ago

OK I have a minimal-ish reproducer for this, but apologies I didn't turn it into a JSweet unit test. I looked at the unit test layer and see how it's structured. When I have a bit more time maybe I can get back to digging into that. In any case, here is the reproducer:

https://github.com/EricWittmann/jsweet-reproducer-574

To see the output I'm observing, do this:

mvn clean install

Then look at target/ts/src/io/example/reproducer/impl/ReproducerFactory.ts and you'll see this:

/* Generated from Java with JSweet 2.3.6 - http://www.jsweet.org */
import { Reproducer } from '../Reproducer';
import { ReproducerTypeWrapper } from '../ReproducerTypeWrapper';

/**
 * @author eric.wittmann@gmail.com
 * @class
 */
export class ReproducerFactory {
    public static createReproducer(wrapper : ReproducerTypeWrapper) : Reproducer {
        switch((wrapper.getType())) {
        case ReproducerType.alpha:
            return new AlphaReproducer();
        case ReproducerType.beta:
            return new BetaReproducer();
        default:
            throw Object.defineProperty(new Error("Failed to create a reproducer: " + wrapper.getType()), '__classes', { configurable: true, value: ['java.lang.Throwable','java.lang.Object','java.lang.RuntimeException','java.lang.Exception'] });
        }
    }
}
ReproducerFactory["__class"] = "io.example.reproducer.impl.ReproducerFactory";

import { BetaReproducer } from './BetaReproducer';

import { AlphaReproducer } from './AlphaReproducer';

Notice two things about this:

lgrignon commented 4 years ago

Thanks, this will help

renaudpawlak commented 4 years ago

I will take a deep look at it ASAP since this regression is my fault. Thanks a lot for the reproducer project.

To let you know, imports at the end of the file do work and they are a way to avoid obvious circular dependency issues. I have to admit this is kind of a hack... so I should probably rethink it as not the default behavior.

lgrignon commented 4 years ago

Thanks @renaudpawlak @EricWittmann I just redeployed 2.3.7-SNAPSHOT with maven plugin. Could you please give it a try on your project?

EricWittmann commented 4 years ago

Checking this out now. :)

EricWittmann commented 4 years ago

OK - my project builds successfully with 2.3.7-SNAPSHOT. Thanks for the fix. Dependabot and I look forward to the final 2.3.7 version. :)

lgrignon commented 4 years ago

Hey, FYI 2.3.7 is released :) It will be sync on maven central as well soon.

EricWittmann commented 4 years ago

Thanks!