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

Fable 4 update (WIP) #15

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

PaigeM89
Copy link

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (fake build or .\build.cmd) on local branch was successful

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

This project cannot be used on Fable 4 projects.

What is the new behavior?

I wasn't able to split this into smaller pieces; updating to Fable 4 wasn't too bad, but verifying the rest of the build was a bit of work just to make sure the changes were stable.

This PR changes a few things:

  • The dependency on Fable.Extras has been removed, as that library is using Fable 3
  • This project has been updated for Fable 4 & dotnet 7
  • The build script has been moved to a build project as the FAKE runner isn't very stable these days
  • The build tooling was updated to FAKE 6
  • I broke linting & formatting
  • I commented out a couple of bindings because they were throwing errors and I couldn't find them in the recoil docs - very possible I just missed something though

Minor changes:

  • I had to tweak the Regex in the docs to remove a warning due to unicode encoding & escaping

Does this introduce a breaking change?

  • Yes
  • No

Existing applications should upgrade to Fable 4 if possible.

Other information

I did not thoroughly test the bridge components.

Let me know what additional changes you'd like to see before we merge this - I'd really like to use it!

@@ -403,9 +407,14 @@ let main = React.memo(fun () ->
]
])

let render' = React.memo(fun () ->
[<ReactComponent>]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future PR I'll update all the components to use this for function components

@MarkBracke
Copy link

@Shmew - are you still working on this library?

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.

2 participants