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

fix: resolve the error caused by named import #1

Merged
merged 2 commits into from
Sep 2, 2024
Merged

fix: resolve the error caused by named import #1

merged 2 commits into from
Sep 2, 2024

Conversation

jinpill
Copy link
Contributor

@jinpill jinpill commented Aug 29, 2024

Hello, 👋🏻

I discovered your project while looking for a solution to the non-functioning electron-push-receiver. Your project seemed relevant to what I needed.

However, when trying to use it in my Electron code, I encountered the following error:

App threw an error during load
file:///Users/metamorp/Desktop/projects/test/fcm/firebase-electron-test/node_modules/firebase-electron/dist/index.js:223
import { load, BufferReader } from "protobufjs";
               ^^^^^^^^^^^^
SyntaxError: Named export 'BufferReader' not found. The requested module 'protobufjs' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'protobufjs';
const { load, BufferReader } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:134:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:217:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async loadApplicationPackage (file:///Users/metamorp/Desktop/projects/test/fcm/firebase-electron-test/node_modules/electron/dist/Electron.app/Contents/Resources/default_app.asar/main.js:129:9)
    at async file:///Users/metamorp/Desktop/projects/test/fcm/firebase-electron-test/node_modules/electron/dist/Electron.app/Contents/Resources/default_app.asar/main.js:241:9

To resolve this, I modified how the protobufjs module is imported. After these changes, all existing tests are passing successfully.

I've submitted these changes for your review. If you find them suitable, please consider incorporating them into the project.

Your feedback on this would be appreciated.

Thank you!

To resolve an error that occurs when importing the setup method in an mjs file, the way of importing the protobuf module has been modified.
@rixcian
Copy link
Owner

rixcian commented Aug 30, 2024

Hey @jinpill ,

first, thank you for your interest in the project.

Yesterday, the library wasn't still ready for an official release. A few minutes ago I released a first official release (v1.0.0). Hopefully, everything should be now functional for you.

Already released on NPM: https://www.npmjs.com/package/firebase-electron

Please try it and let me know if the problem still persists.

@jinpill
Copy link
Contributor Author

jinpill commented Sep 2, 2024

Thank you for reviewing my pull request.

I just updated to v1.0.1 and tested it, but it still does not work.
This issue seems to occur when the file extension is mjs; it does not occur when the extension is js.

I have attached a link to a simple example project on GitHub where you can see the error: https://github.com/jinpill/firebase-electron-mjs

If you cannot replicate the issue with this project, please let me know.

@rixcian
Copy link
Owner

rixcian commented Sep 2, 2024

I've successfully replicated the bug and the fix, so I'm merging and releasing it.

Thank you for finding and fixing the bug. I appreciate it very much <3

@rixcian rixcian merged commit 76473b1 into rixcian:master Sep 2, 2024
1 check failed
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