Skip to content

Update lodash reference in vite optimizeDeps demo configs #123

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

Merged
merged 7 commits into from
Apr 29, 2024

Conversation

aguynamedloren
Copy link
Contributor

Hey folks!

I ran into a bit of trouble during setup with a new codebase, using powersync-sdk-web, bundled with vite. I modeled my vite config after the demo config here. Upon page load, the following error was displayed in the browser console:

Uncaught SyntaxError: The requested module '.../lodash/throttle.js' does not provide an export named 'default' (at AbstractStreamingSyncImplementation.js)

I was able to resolve the issue by changing the vite config lodash depdency line to lodash/throttle. I suspect this may've been a regression with the recent lodash optimization. I'm new to vite, not sure if this is the best solution, but it worked for me, and I hope it'll help others hitting the same wall I ran into.

Appreciate the great work you're doing here!

@Chriztiaan
Copy link
Contributor

Chriztiaan commented Apr 16, 2024

Thanks for this. There has been another reported issue with a project setup where someone had to include the nested dependency. When I tested this on my side in a standalone repo I was only able to get this working with the nested definition.

Could I ask whether you could test the following in your project? Just want to make sure it doesn't break anything:

  include: [
      "event-iterator",
      "uuid",
      "js-logger",
      "lodash/throttle",
      "can-ndjson-stream",
      "@journeyapps/powersync-sdk-web > event-iterator",
      "@journeyapps/powersync-sdk-web > uuid",
      "@journeyapps/powersync-sdk-web > js-logger",
      "@journeyapps/powersync-sdk-web > lodash/throttle",
      "@journeyapps/powersync-sdk-web > can-ndjson-stream",
    ],
    ```

@aguynamedloren
Copy link
Contributor Author

aguynamedloren commented Apr 16, 2024

@Chriztiaan thanks for the quick reply! Yeah, that config worked for me -- didn't break anything.

I wasn't familiar with the nested dependency syntax until you shared it; my intuition is that it's probably the better approach.

Out of curiosity, I also tried with nested dependency declarations only. It almost worked, just had to change uuid path:

    include: [
      '@journeyapps/powersync-sdk-common > uuid',
      '@journeyapps/powersync-sdk-web > event-iterator',
      '@journeyapps/powersync-sdk-web > js-logger',
      '@journeyapps/powersync-sdk-web > lodash/throttle',
      '@journeyapps/powersync-sdk-web > can-ndjson-stream'
    ]

Edit: I should add that my testing is extremely minimal here. I'm essentially just ensuring that vite bundles, the page loads in the browser without errors, and powersync requests are successful.

@Chriztiaan
Copy link
Contributor

Thanks for checking! I'll fiddle a bit and test it on my side.

@Chriztiaan
Copy link
Contributor

I've updated the diff to use the nested dependency syntax. Thanks for the help.

@Chriztiaan Chriztiaan removed the request for review from stevensJourney April 16, 2024 14:18
stevensJourney
stevensJourney previously approved these changes Apr 17, 2024
@Chriztiaan
Copy link
Contributor

Chriztiaan commented Apr 25, 2024

Updated this after our package renaming @journeyapps/powersync-xxx -> @powersync/xxx.
Verified that this works without needing a pnpm overrides entry for uuid after bumping all the underlying UUID versions in #130.

Copy link
Collaborator

@stevensJourney stevensJourney left a comment

Choose a reason for hiding this comment

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

Thanks!

@Chriztiaan Chriztiaan merged commit 371e8ce into powersync-ja:main Apr 29, 2024
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