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

Web support vol.1 #189

Merged
merged 5 commits into from
Mar 8, 2021
Merged

Web support vol.1 #189

merged 5 commits into from
Mar 8, 2021

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Feb 5, 2021

Part of #185

  • Shuffles stuff around and uses conditional exports to add "support" JS/web.
  • Actual support not implemented yet, just won't import anything not available on JS/web platforms, thus the analyzer thinks package supports it.
  • I've done my best to make this reviewable commit by commit. Ignore "Files changed" tab in the PR, it doesn't show up the moved files properly.

TODO:

  • Don't merge as is, we should actually add some import that breaks web support before releasing, to avoid user confusion. E.g. add the following code to lib/objectbox.dart: import 'dart:ffi';
  • ALso for the same reason don't merge the CI commit, it's here just so that we don't break the web support unintentionally when changes are made before merging

@vaind vaind requested a review from greenrobot-team February 5, 2021 18:24
@vaind vaind added this to the 1.0 milestone Feb 5, 2021
@greenrobot-team
Copy link
Member

@vaind
Copy link
Contributor Author

vaind commented Feb 8, 2021

Note: Dart docs about conditional imports https://dart.dev/guides/libraries/create-library-packages#conditionally-importing-and-exporting-library-files

Make sure to read the linked issue in case you haven't (the link's there)

Copy link
Member

@greenrobot-team greenrobot-team left a comment

Choose a reason for hiding this comment

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

LGTM, no obvious issues, still builds and tests run on my Windows setup.

I'd imagine not having an "interface" or stub implementation could lead to deviations in the two APIs. Does the compiler somehow check the APIs match between implementations? But I guess tests would uncover this quickly.

Also having to copy some of the generated constants is a step back, but I don't see another solution as well.

@vaind vaind merged commit 85e0ced into main Mar 8, 2021
@vaind vaind deleted the web-support-1 branch March 8, 2021 07:22
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