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

Renaming issues when dealing with tink #10

Closed
Tobbse opened this issue Oct 25, 2024 · 1 comment
Closed

Renaming issues when dealing with tink #10

Tobbse opened this issue Oct 25, 2024 · 1 comment

Comments

@Tobbse
Copy link

Tobbse commented Oct 25, 2024

Hello!
I'm really glad to see the recent improvements to haxe-rename. I am now using the latest version of vshaxe 2.32.2 and it works much better that it did before for me. Thanks! 👍

However, I have noticed that there are some special cases where the renaming still does not work for me.
There's quite a lot of tink async handling in our project and there seems to be an issue with renaming usages inside callback functions (e.g. when resolving futures and promises).

I can reliably reproduce this in an example I have created:




To break it, just rename Printer.hx and observe that the first 2 example usages are renamed, but the other 2 are not.

The example is just one haxe module but this also fails when renaming across different modules. In this case the import is updated, but the usages still break.

image



Also attaching the code again just in case:

import js.Browser;
import tink.core.Future;
import tink.core.Promise;

@:expose class Main {
	public static function main():Void {
		// Setup stuff
		final futureTrigger = Future.trigger();
		futureTrigger.trigger("Future");

		final promise:Promise<String> = Promise.resolve("Promise");

		// Example 1 - "Normal" usage of `Printer` (WORKS)
		new PrinterTestRename("Normal");

		// Example 2 - `Printer` used within promise handling (WORKS)
		promise.handle(result -> {
			switch (result) {
				case Success(text): new PrinterTestRename(text);
				default:
			}
		});

		// Example 3 - `Printer` used within future handling (FAILS)
		futureTrigger.asFuture().handle(text -> {
			new Printer(text);
		});

		// Example 4 - `Printer` used within promise handling but promise is created in `TextLoader` (FAILS)
		new TextLoader().load("TextLoader").handle(result -> {
			switch (result) {
				case Success(text): new Printer(text);
				default:
			}
		});
	}
}

// Rename this class and observe that examples 1 and 2 are updated, but the others are not.
class PrinterTestRename {
	public function new(text:String) {
		Browser.console.log(text);
	}
}

class TextLoader {
	public function new() {}

	public function load(text:String):Promise<String> {
		return Promise.resolve(text);
	}
}
@Tobbse
Copy link
Author

Tobbse commented Oct 31, 2024

thanks for the quick fix! ❤️

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

1 participant