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

Fixed platform selection in loadTsConfig.ts #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SimonSimCity
Copy link
Contributor

os.platform is a function (https://nodejs.org/api/os.html#osplatform), returning a string.

Fixes #20

@t3chguy
Copy link

t3chguy commented Jan 19, 2024

@mighdoll in case this slipped through the cracks, users of electron-builder (a downstream of yours) would really appreciate this landing and being released :) keep up the good work

@awdr74100
Copy link

@mighdoll releasing it will give the electron-builder config good type support! Let's do it!

@mighdoll
Copy link
Owner

hmm... tests don't pass with this on the windows machine I just fired up. do they work for you? pnpm test

@mighdoll
Copy link
Owner

the integration test 'loading a config file' does pass on windows with this patch, so very possibly the test failures are a separate issue.

@awdr74100 @SimonSimCity have you been using the patched version successfully? If so, we can fix the tests separately.

@CatStudioApp
Copy link

the integration test 'loading a config file' does pass on windows with this patch, so very possibly the test failures are a separate issue.

@awdr74100 @SimonSimCity have you been using the patched version successfully? If so, we can fix the tests separately.

We've been using the patched version for a while of this PR.

# Script to modify index.js due to <https://github.com/mighdoll/config-file-ts/issues/19>

# Get current path of this script
$currentPath = $PSScriptRoot

# Define the relative path to the index.js file
$relativePath = "../../../../../web/node_modules/config-file-ts/dist/index.js"

# Combine the paths to get the full path to index.js
$indexPath = Join-Path $currentPath $relativePath

# Check if the file exists
if (Test-Path $indexPath) {
    # Read the contents of the file
    $fileContent = Get-Content $indexPath -Raw

    # Replace the specified string
    $fileContent = $fileContent -replace 'os.platform.name', 'os.platform()'

    # Write the changes back to the file
    Set-Content $indexPath -Value $fileContent

    Write-Host "File updated successfully."
} else {
    Write-Error "File not found: $indexPath"
}

@mighdoll
Copy link
Owner

ok, thanks good to know that it's working for you. I'm adding another test too to verify that the cache works.

@mighdoll
Copy link
Owner

btw, do you know about 'patch-package'? It's handy for tweaking upstream packages like this. Someone showed it to me a while ago and I've found it useful, so I thought I'd pass the tip along in case you hadn't seen it.

@t3chguy
Copy link

t3chguy commented Feb 17, 2024

btw, do you know about 'patch-package'? It's handy for tweaking upstream packages like this. Someone showed it to me a while ago and I've found it useful, so I thought I'd pass the tip along in case you hadn't seen it.

It isn't intended for use in published packages ds300/patch-package#198

@mighdoll
Copy link
Owner

ah, good point

@mighdoll
Copy link
Owner

published 0.2.8-rc1.

Let me know if that works!

@t3chguy
Copy link

t3chguy commented Apr 8, 2024

@mighdoll can confirm it works - used resolutions to eagerly update config-file-ts in element-hq/element-desktop#1591 - Thanks!

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.

None yet

5 participants