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

Don't ignore error when loading plugin #3441

Closed
MangelMaxime opened this issue May 5, 2023 · 5 comments
Closed

Don't ignore error when loading plugin #3441

MangelMaxime opened this issue May 5, 2023 · 5 comments
Labels

Comments

@MangelMaxime
Copy link
Member

MangelMaxime commented May 5, 2023

Description

In the past, I had the feeling that the plugin version compatibility detection is off and this elmish/react#87 seems to confirm it.

I feel like if the version are not compatible instead of crashing Fable still compile the code but without taking into consideration the plugin. Leading to situation where the code is compiling but incorrect because plugin is not applied.

This issue needs further investigation but I wanted to log it so it can be tracked.

Related information

  • Fable version: I think Fable 3 and Fable 4 are impected
@MangelMaxime
Copy link
Member Author

Using Fable 4 with a plugin expecting version 3.0 fail as expected (I think Fable doesn't expect to support the same AST between major versions) ✅

CleanShot 2023-05-05 at 17 29 46

Using Fable 3 with a plugin expecting version 4.0 succeeds instead of failing ❌

CleanShot 2023-05-05 at 17 34 15

I suppose the problem is also present in Fable 4 if we have a plugin expecting version 5.0. So both Fable 4 and Fable 3 needs a fix, in order to improve the transitioning between major releases.

@MangelMaxime
Copy link
Member Author

After running a local version of Fable, the problems doesn't seems to be with the version detection but because when executing Fable 3 with Feliz 4 there are 0 plugins detected.

And so, because no plugin is detected the version check never happens.

I am exploring why the plugins are not detected.

@MangelMaxime
Copy link
Member Author

@alfonsogarciacaro I implemented a fix for Fable 3 but I think something similar needs to be ported to Fable 4.

Perhaps, by trying to give more context on the error side to have a better error message than the one I made for Fable 3.

@MangelMaxime MangelMaxime changed the title Detection of plugin version compatibility seems off Don't ignore error when loading plugin May 11, 2023
@alfonsogarciacaro
Copy link
Member

Thanks @MangelMaxime! Yes, probably we can port your code to Fable 4, or make the plugin loader also reject new major versions. Can you please send a new PR for main branch?

@MangelMaxime
Copy link
Member Author

or make the plugin loader also reject new major versions

I agree doing that is best. And actually, Fable kind of does that later in the code when trying to apply the plugin.

The problem, in the case I covered for Fable 3 was that the code responsible for loading the assembly throw an exception:

image

So we had an error before being able to load the plugin and get access to its version information. I don't know what is causing this load error, perhaps this is because of an incompatible dependency like FSharp.Core or Fable.Core having a different version than he one loaded by Fable.


I can indeed, port my code to the main branch and move the version detection logic at the plugin loading stage too if you want. Because, loading a plugin and failing later to apply it doesn't seem really intuitive. And it is probably better if Fable stop here instead of letting the user think that all is fine unless they read the console logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants