zengm-games / zengm

Basketball GM (and other ZenGM games) are single-player sports management simulation games, made entirely in client-side JavaScript.
https://zengm.com
Other
349 stars 127 forks source link

Fix path resolution for local build on Windows #442

Closed fearandesire closed 1 year ago

fearandesire commented 1 year ago

Addresses the file pathing issue when attempting to compile or start a local build. Added fileURLToPath to convert the file URL to a file path string, which is compatible with both Unix and Windows systems.

I was able to run yarn run build and yarn run start on my local Windows and Linux operating systems successfully after the change.

dumbmatter commented 1 year ago

This is nice!

  1. Can you sign the CLA as described here?

  2. Can you abstract this into a function, so it can be used in the other places __dirname is used currently? There are a few other places using it, if you search the codebase. Maybe create tools/lib/getDirname.js like:

export const getDirname = (url: string) => {
    return = path.dirname(
        path.normalize(fileURLToPath(url)),
    );
}

(Also see if that path.normalize call is really needed?)

And then call it like:

const __dirname = getDirname(import.meta.url);
fearandesire commented 1 year ago

This is nice!

1. Can you sign the CLA as described [here](https://github.com/zengm-games/zengm#license-and-contributor-license-agreement)?

2. Can you abstract this into a function, so it can be used in the other places __dirname is used currently? There are a few other places using it, if you search the codebase. Maybe create tools/lib/getDirname.js like:
export const getDirname = (url: string) => {
    return = path.dirname(
      path.normalize(fileURLToPath(url)),
  );
}

(Also see if that path.normalize call is really needed?)

And then call it like:

const __dirname = getDirname(import.meta.url);

No problem! Implemented those changes in the latest commit. I also removed the path.normalize, you were correct in the thought it wasn't needed.

As for the CLA, I emailed a copy yesterday, 3/21/22 from (fenixcoding@gmail.com) - Would you want me to edit it on my repo as well? I thought I just had to send that email with the information filled out but definitely let me know otherwise/if I misread.

fearandesire commented 1 year ago

In the latest commit, I changed the eslint config '.eslintrc'. While I was checking if eslint was working, VS Code gave me an error that tsconfig.json couldn't be found despite it directly pointing to the file. Just some quick research and I was able to correct the error by adding "tsconfigRootDir": "./". This shouldn't make a single change to the linting, but let me know if you're against the change and I'll try to figure something else out.

dumbmatter commented 1 year ago

Thanks!