powersync-ja / powersync-js

SDK that enables local-first and real-time reactive apps with embedded SQLite for JavaScript clients, including React Native and Web
https://www.powersync.com
Apache License 2.0
300 stars 21 forks source link

Update Supabase demo to warn about `js-logger` when running Vite #267

Closed guillempuche closed 2 months ago

guillempuche commented 2 months ago

Prevent this issue:

Uncaught SyntaxError: The requested module 'http://localhost:3000/@fs/.../<repo>/node_modules/js-logger/src/logger.js?v=eb4a8b9e' doesn't provide an export named: 'default'

Discussion started on Discord https://discord.com/channels/1138230179878154300/1272981660589363293

changeset-bot[bot] commented 2 months ago

⚠️ No Changeset found

Latest commit: 1adcab0472965a5d2d56d938ba67ef2f02ab99ab

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

MrLightful commented 2 months ago

I had to include js-logger even having it installed in package.json. Can you please double check this?

guillempuche commented 2 months ago

In my case, I didn't install it.

MrLightful commented 2 months ago

In my case, I didn't install it.

Yeah well I mean phrasing Only when 'js-logger' isn't installed in package.json suggests that you don't need it when js-logger installed. But it seems that it's needed even when it's installed. So maybe just recommend to always include it? Or when you install it, it's not breaking without include?

guillempuche commented 2 months ago

What about now?

Chriztiaan commented 2 months ago

So turns out you need to install js-logger and configure/apply it, alternatively you can have the include rule. I think we should just add the include rule everywhere for the moment. @guillempuche would you mind applying it to all the vite projects when you have a moment - since you already have this PR going :) ?

guillempuche commented 2 months ago

@Chriztiaan, done.

Chriztiaan commented 2 months ago

Looks like there is work being done on this here to resolve the underlying exclude issue.