Skip to content
This repository was archived by the owner on Nov 24, 2018. It is now read-only.

Windows Build Compatibility #70

Open
Ranger2959 opened this issue Jul 28, 2017 · 5 comments
Open

Windows Build Compatibility #70

Ranger2959 opened this issue Jul 28, 2017 · 5 comments

Comments

@Ranger2959
Copy link
Contributor

I found a couple issues while trying to build this on Windows.

  1. For "npm run build" --> "rm -rf dist; tsc -d"

    • "rm -rf dist;" doesn't make sense on windows since the rm command doesn't exist. The command to remove a directory in windows is "rmdir"
    • I'm open to suggestions on how to solve this one, my only thought is to have 2 different build scripts (one for windows and one for mac/linux)
  2. https://github.com/graphcool/chromeless/blob/master/src/chrome/local-runtime.ts#L204-L205

    • This line sets the tmp directory as "/tmp". On windows this refers to "C:/tmp". I think we should change this line to refer to just "tmp/" so that it refers to a tmp folder inside the current directory.
    • Also, fs.writeFileSync() assumes the tmp folder exists, I think we need code in here to check for the folder and add the folder if it doesn't exist.

Once we have solid solutions to these 2 things, I can implement a PR.

@adieuadieu
Copy link
Collaborator

If we want complete support for Chromeless on Windows, including the Proxy, there are further challenges. There is an issue upstream in the serverless-chrome package (adieuadieu/serverless-chrome#21) which is blocked by an issue upstream from that in Serverless (serverless/serverless#3557).

@Ranger2959
Copy link
Contributor Author

Ok, makes sense. But I still think at least Issue 2 (assuming /tmp exists) is an issue that is visible on all platforms and should be fixed?

@adieuadieu
Copy link
Collaborator

@Chrisgozd definitely. We don't have to wait for the upstream issues to be resolved to address your two points. 😄 Luckily the npm package script on Windows issue has popular solutions provided by the community:

  1. Instead of rm -rf we could use rimraf. tsc is a command line tool installed by the typescript package and lives in the project's node_modules/.bin. I think it should be there on Windows, too. So the final build script might be rimraf dist/ && tsc -d. If that doesn't work, then there's cross-env or npm-better-run. cross-env is nice because you don't have to add yet-another-section to the package.json.
  2. As far as I'm aware, /tmp always exists on Unix-based OSs. However, here's a solution for the tmpdir for both Windows and *nix. I specifically like that they create a temporary folder within the OSs temp directory.

@joelgriffith
Copy link
Contributor

Also should look out for referencing paths with "/" notation as well. path.join all the things

@adieuadieu
Copy link
Collaborator

Making a note here: @joelgriffith's #97 will address the first point (rimraf).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants