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

Separate Result from Choice #3739

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Conversation

ncave
Copy link
Collaborator

@ncave ncave commented Feb 6, 2024

  • Separate Result and Choice modules.

@MangelMaxime
Copy link
Member

Makes sense, I believe this will be a breaking change for fable-library NPM packages.

How should we handle that, should fable-library follows SemVer strictly or should we say that [email protected] is for Fable 4 and [email protected] is for Fable 5?

In this case we would use the offset from one level the standard:

  • Standard is Major.Minor.Patch
  • And here we would have FableVersion.Major.Minor

I believe FableVersion.Major.Minor could make senses because it is easer to reason about that thinking [email protected] and [email protected] supports Fable 4 for example.

Pinging @nojaf because he is using fable-library.

@nojaf
Copy link
Member

nojaf commented Feb 6, 2024

I would be ok with fable-library not adhering to SemVer. This was an initial concern (I cannot find the original issue) and there I believe there it was agreed that we would not be burdened with it.
Align the version with Fable.Core NuGet, if it breaks it breaks.
And how about publishing a new version in the first place 😉

@MangelMaxime
Copy link
Member

And how about publishing a new version in the first place 😉

I see what you are trying to do here 😏

#3614

I "concede", I will work on releasing a new version of it this week. I just need to decide between @fable-community and @fable-org for the org name. I am personally leaning towards @fable-org as it feels a bit more "official".

@nojaf
Copy link
Member

nojaf commented Feb 6, 2024

I am personally leaning towards @fable-org as it feels a bit more "official".

Then by all means: go with @fable-org. I suspect @ncave and @dbrattli don't have a strong opinion on that subject either.

@MangelMaxime
Copy link
Member

Align the version with Fable.Core NuGet, if it breaks it breaks.

I don't think that fable-library is linked to Fable.Core version it is linked to Fable itself directly.

In fact, Fable.Core is more behaving like fable-library. Fable.Core works for specifics version of Fable.

Up to now, we were "lucky" (because we avoided moving things around too much) that Fable.Core 2+, 3+, 4+ works with Fable upward version.

I am preparing the changes for the next release of Fable, and I think this PR is green so we can merge.

@MangelMaxime MangelMaxime merged commit c8d7724 into fable-compiler:main Feb 6, 2024
19 checks passed
@nojaf
Copy link
Member

nojaf commented Feb 6, 2024

As far as my understanding goes fable-library is the compiled version of Fable.Core.
Used by the compiler, yes. But if an fsproj doesn't have a reference to Fable.Core it will use the one Fable.Cli references.

@ncave
Copy link
Collaborator Author

ncave commented Feb 6, 2024

@MangelMaxime

I don't think that fable-library is linked to Fable.Core version it is linked to Fable itself directly.

Yes, I agree fable-library versioning has to align with Fable versioning. You don't have to publish a new fable-library release with every Fable release (although it would be nice if possible to automate), but definitely needed when fable-library changes.

@nojaf

As far as my understanding goes fable-library is the compiled version of Fable.Core.

Not exactly :) Fable.Core technically is just JavaScript bindings. It doesn't compile to any code.
Where fable-library is somewhat equivalent to FSharp.Core. I know, somewhat confusing.

@MangelMaxime I have no preference on the npmjs org name. And perhaps using fable-library-js can help differentiate it from the existing package.

@MangelMaxime
Copy link
Member

fable-library is not the compiled version of Fable.Core they are 2 distincts entities.

fable-library is just a way for us to write F# features using native code.

For example, we have https://github.com/fable-compiler/Fable/blob/main/src/fable-library/Async.ts which implements the Async support from F#/BCL for TypeScript/JavaScript target.

Fable.Core is a dll which provide binding for common JavaScript API/Types and also custom Fable types/attributes/functions. For example, it contains ImportAttributes, JS.console.log, etc.

@MangelMaxime
Copy link
Member

MangelMaxime commented Feb 6, 2024

@MangelMaxime I have no preference on the npmjs org name. And perhaps using fable-library-js can help differentiate it from the existing package.

My goal is to move all of existing Fable NPM packages under the new organisation like that everything is centralised #fable-library. It also means that I give invite people in the @fable-org NPM organisation and any new packages published will be accessible to those people too. In case, I decide to not work on Fable anymore or something like that. This should avoid the situation we have right now where only Alfonso can publish fable-lbrary.

@ncave
Copy link
Collaborator Author

ncave commented Feb 6, 2024

@MangelMaxime I get that, I was just saying the package name difference can perhaps help when searching for it in npmjs, that's all.

@MangelMaxime
Copy link
Member

@MangelMaxime I get that, I was just saying the package name difference can perhaps help when searching for it in npmjs, that's all.

Ah yes got it, will think about it.

@MangelMaxime
Copy link
Member

Actually, could make sense as perhaps we need @fable-org/fable-library-js and @fable-org/fable-library-ts 🧐

@nojaf
Copy link
Member

nojaf commented Feb 6, 2024

Where fable-library is somewhat equivalent to FSharp.Core. I know, somewhat confusing.

Thanks for explaining that one @ncave. I always assumed Fable.Core was both the implementation of F# and BCL stuff and some helper bindings for JS.

@ncave
Copy link
Collaborator Author

ncave commented Feb 6, 2024

For Rust we're not currently publishing fable-library-rust to a package repo (although it might be a good idea to stake out the name in case we change our minds in the future).
The reason being it changes so often that the most correct fable-library-rust version is the one packaged with Fable itself.

Just to make sure I understand all the use cases of publishing fable-library, is it just to avoid packaging it with compiled code, to (hopefully) save some space when referencing other pre-compiled packages that hopefully use the same version of fable-library? Or is there something else I'm missing?

@ncave
Copy link
Collaborator Author

ncave commented Feb 6, 2024

@MangelMaxime If you do decide to go with fable-library-js, we might as well rename the src/fable-library folder too, for consistency.

@ncave ncave deleted the result branch February 6, 2024 16:56
@MangelMaxime
Copy link
Member

For Rust we're not currently publishing fable-library-rust to a package repo (although it might be a good idea to stake out the name in case we change our minds in the future).
The reason being it changes so often that the most correct fable-library-rust version is the one packaged with Fable itself.

Yes for Rust and Python, I think right now it is too early to publish them on their respective dependency manager because they are not stable enough yet.

Just to make sure I understand all the use cases of publishing fable-library, is it just to avoid packaging it with compiled code, to (hopefully) save some space when referencing other pre-compiled packages that hopefully use the same version of fable-library? Or is there something else I'm missing?

Yes there is this reason and another one.

Currently, in Fable JavaScript (I don't know for others target) if you have the following architecture:

  1. ProjectA publish as NPM packages
  2. ProjectB publish as NPM packages
  3. A third package which use project-a and project-b then you can have some equality that you would expect to succeed fails because the constructors is not coming from the same class. (ProjectA and ProjectB each have their own definition of fable-library inlined in them).

If you try to compare (using F# comparaison) an instance of types coming from fable-library created from inside of ProjectA and ProjectB even if they are the same "types" with the same value their equality will fail.

I made a project to demonstrate this behaviour here:

https://github.com/MangelMaxime/demo-why-using-fable-library-can-improve-equality

This is for example what is driving this issue too #3737

@ncave
Copy link
Collaborator Author

ncave commented Feb 6, 2024

@MangelMaxime Yes, well, that's because of the particular way structural equality is implemented for JavaScript right now. It relies on the object constructor function being the same. But that would only work if both packages reference the same fable-library version, which they might not.

Nonetheless, I got your point, thanks for elaborating.

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

Successfully merging this pull request may close these issues.

3 participants