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

Enhancement or Bug: Handling of 'electron/*' process-specific module aliases #372

Closed
4 tasks done
undergroundwires opened this issue Jan 5, 2024 · 2 comments
Closed
4 tasks done
Labels
bug Something isn't working enhancement New feature or request

Comments

@undergroundwires
Copy link

undergroundwires commented Jan 5, 2024

Describe the bug

Hi @alex8088, thank you again for the project, everything has been painless so far. Your efforts are appreciated to keep it up to speed with latest changes in ecosystem (vite, ESM etc.).

I have an idea for improvement (or maybe a bug depends on how we look at it).

While importing from electron directly works fine (e.g., import { app } from 'electron'), I've noticed that alias imports like electron/main, electron/common (which is considered best practice as it scopes the import to the right process), such as import { app } from 'electron/main' does not work.

Error:

[vite]: Rollup failed to resolve import "electron/main" from ".../privacy.sexy/src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`

Currently, I can work around this issue by explicitly including 'electron/main' in the plugin configuration:

plugins: [externalizeDepsPlugin({
      include: ['electron/main', 'electron/common', 'electron/renderer'],
    })],

Or I can do that using using resolve.alias:

resolve: {
      alias: {
        'electron/main': 'electron',
        'electron/common': 'electron',
        'electron/renderer': 'electron',
      },
    },

Using resolve.alias is better because it also works in non-electron builds for example when building unit tests.

Would it be beneficial to provide this config through electron-vite somehow? Part of externalizeDepsPlugin or something similar configuration that can be shared across electron.vite.config.ts and unit tests.

It's probably nice-to-have but it will have good impact on developer experience.

Electron-Vite Version

1.0.28

Electron Version

27.0.0

Vite Version

4.4.11

Validations

@undergroundwires undergroundwires changed the title Enhancement or Bug: Handling of 'electron/main' in externalizeDepsPlugin Enhancement or Bug: Handling of 'electron/*' in externalizeDepsPlugin Jan 5, 2024
@undergroundwires undergroundwires changed the title Enhancement or Bug: Handling of 'electron/*' in externalizeDepsPlugin Enhancement or Bug: Handling of 'electron/*' process-specific module aliases Jan 5, 2024
@alex8088 alex8088 added bug Something isn't working enhancement New feature or request labels Jan 6, 2024
@alex8088
Copy link
Owner

alex8088 commented Jan 6, 2024

Hi, @undergroundwires, Thx. This is a bug, Electron's export subpaths also need to be externalized. I'll fix it soon.

@alex8088
Copy link
Owner

alex8088 commented Jan 9, 2024

electron-vite 2.0 is out! 🎉, fixed

@alex8088 alex8088 closed this as completed Jan 9, 2024
undergroundwires added a commit to undergroundwires/privacy.sexy that referenced this issue Mar 18, 2024
This commit bumps Electron and related dependencies to their latest
versions to leverage native ESM support. It adjusts build configuration
to use native ESM support instead of relying on CommonJS bundling.

Key changes:

- Bump Electron to latest v29.
  Electron v28 ships with native ESM/ECMAScript modules support.
  Details on Electron ESM support:
    - electron/electron#21457
    - electron/electron#37535
- Bump `electron-builder` to latest v24.13.
  `electron-builder` is used to package and publish the application.
  It supports ESM since 24.10.
  Details on `electron-builder` ESM support:
    - electron-userland/electron-builder#7936
    - electron-userland/electron-builder#7935
- Bump `electron-log` to latest v5.1.
  `electron-log` supports ESM since version 5.0.4.
  Details on `electron-log` ESM support:
    - megahertz/electron-log#390.
- Change `electron-vite` configuration to bundle as ESM instead of
  CommonJS to leverage Electron's native ESM support.

Other supporting changes:

- Add type hint for electron-builder configuration file.
- Update import statements for `electron-updater` as it still is a
  CommonJS module and does not support ESM.
  Details:
    - electron-userland/electron-builder#7976
- Improve `electron-builder` configuration file to dynamically locate
  main entry files, supporting various JavaScript file extensions
  (`.js`, `.mjs` and `.cjs`) to facilitate easier future changes.
- Change comment about Electron process-specific module alias
  registration. This issue has been fixed in `electron-vite`, but
  subpath module imports for Electron still do not work when building
  tests (`npm run test:unit`).
  Details:
   - alex8088/electron-vite#372
- Add `electron-log` in bundling process instead of externalizing to
  workaround Electron ESM loader issues with subpath imports (inability
  to do `electron-log/main`).
  Details:
    - alex8088/electron-vite#401
    - electron/electron#41241
- Improve desktop runtime error checks' assertion message for better
  clarity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants