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

Bug: default imports for built-in node modules in electron-updater #6134

Closed
Thomb89 opened this issue Aug 9, 2021 · 1 comment
Closed

Bug: default imports for built-in node modules in electron-updater #6134

Thomb89 opened this issue Aug 9, 2021 · 1 comment
Labels

Comments

@Thomb89
Copy link

@Thomb89 Thomb89 commented Aug 9, 2021

  • Electron-Builder Version: 22.11.11
  • Node Version: 16.6.1
  • Electron Version: 13.1.8
  • Electron Type (current, beta, nightly): current
  • Target: Windows

There is a bug in the electron-updater package in the file BaseUpdater.ts.

It was introduced recently with this commit: 45fc0a0.

The NodeJS Modules fs and path have no default imports

import fs from "fs"
import path from "path"

should be

import * as fs from "fs"
import * as path from "path"

or

import { writeFileSync, rmSync } from "fs"
import { join, dirname } from "path"

Because of this, it will always throw an error and set installPathRequiresElevation = true.
This is not needed, when installing in the %appdata% directory of the user.
For users without Admin-Rights, this will even install the update for the wrong user.

I think this slipped through because of the typescript compiler option "allowSyntheticDefaultImports".
With this flag

import path from "path"

path.join(...)

compiles to

const path = require('path')

path.default.join(...)
@github-actions github-actions bot added the bug label Aug 9, 2021
burnhamup added a commit to burnhamup/electron-builder that referenced this issue Aug 10, 2021
Fix import errors for fs and path so that the test for admin priviliges functions

Closes electron-userland#6134
@burnhamup
Copy link
Contributor

@burnhamup burnhamup commented Aug 10, 2021

I just noticed this too, trying to test the fix for the silent updater. I submitted a pull request for the import fix.

Loading

mmaietta pushed a commit that referenced this issue Aug 11, 2021
* fix(electron-updater): fix import errors for fs and path so that the test for admin privileges works correctly. Fixes #6134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants