search

elm / compiler

Compiler for Elm, a functional language for reliable webapps.
https://elm-lang.org/
BSD 3-Clause "New" or "Revised" License
7.48k stars 658 forks source link

Free the npm package from third party dependencies #2287

Closed lydell closed 9 months ago

lydell commented 1 year ago

Status update

This is now merged, and released to npm as version 0.19.1-6!

The @lydell/elm package continues to exist as an “unofficial elm package”. The only difference is that it ships more binaries than the official ones.

npm install @lydell/elm

If you use elm-webpack-loader, which depends on the elm npm package, you can use the following package.json to override elmto @lydell/elm:

{
  "overrides": {
    "elm": "npm:@lydell/elm@0.19.1-14"
  },
  "dependencies": {
    "elm-webpack-loader": "^8.0.0"
  }
}

Linked issues

Background

Me, @supermario and @evancz (with the help of some more folks) have been working on a new version of the elm npm package.

We have successfully tested all of the above using a temporary npm package (@lydell/elm): https://discourse.elm-lang.org/t/help-test-the-new-npm-elm-package/8761. elm-format is also successfully using this npm package technique: https://github.com/avh4/elm-format/pull/781

This PR is the result of my work. If you want to avoid getting into Linux ARM compilation, it’s easy to remove the Linux ARM package.

The current elm npm package

The current elm npm package downloads the Elm binary for the current platform using the request package. If there is no binary for the current platform, it exits with a helpful message.

Upsides of this approach:

Downsides of this approach:

The reason for using request rather than Node.js’ https module directly, is because request supports the HTTPS_PROXY environment variable, which is important to people behind corporate proxies or firewalls.

The new elm npm package

This PR switches to a different technique for distributing the binaries.

The binary packages declare which OS and CPU they are compatible with. For example:

  "os": [ "darwin" ],
  "cpu": [ "arm64" ]

The main npm package depend on the binary packages via optional dependencies:

    "@evancz/elm_darwin_arm64": "0.19.1-0",
    "@evancz/elm_darwin_x64": "0.19.1-0",
    "@evancz/elm_linux_arm64": "0.19.1-0",
    ...

When installing, npm fetches the metadata for all the optional dependencies and only installs the one with a matching OS and CPU. If none of them match, npm still considers the install successful. The main npm package still contains an install script that gives a helpful error.

Upsides of this approach:

Downsides of this approach:

Practically, none. No issues were reported during the test run of @lydell/elm, and none have been reported during the 9 months of usage of that package since then either. For completeness, here are the factual differences to the current npm package:

What stayed the same

In short: Most things. The main difference is that the new elm npm package gets the Elm binary from other npm packages instead of from GitHub Releases.

Diff tip

I renamed download.js to binary.js and then edited it. By default, git considers a file to be renamed if there’s at least 50 % similarity. However, binary.js is less than 50 % similar to download.js so the GitHub diff shows download.js as deleted and binary.js as a new file.

Locally you can run:

git diff --merge-base master --find-renames=30%

And then type /download.js and press Enter to jump to the relevant part. I think that diff helps to see that the new approach isn’t all that different from before.

Things left out from this PR

The approach of this PR was inspired by esbuild:

If you read through those files, you’ll see that they are much more elaborate than binary.js in this PR.

Aside about unsupported platforms @alshdavid posted in `#core-coordination` on Slack, which got the ball rolling on this work: > Hi Elm team, David here. I am a contributor of Parcel and we are having the issue where the recently introduced `verifyPlatform()` function in elm/compiler is preventing the Parcel monorepo (contributors) and Parcel+Elm (consumers) from being `npm install`ed when running on `linux-arm64` devices. > > This is an increasingly common use-case with Parcel contributors and consumers due to the rise of M1 MacBooks (Docker & VMs) and Linux ARM hosts being used more frequently as build agents. > > **For contributors of Parcel:** The Parcel monorepo fails to `npm install` at all on `linux-arm64` hosts, which is pretty critical given the use of Docker in development. > > **For consumers of Parcel and Elm:** Projects using Parcel and Elm and are building on `linux-arm64` machines are unable to `npm install`/build their projects. This happens locally when developers are using Parcel+Elm in Docker on an M1 MacBook and happens on build agents, but there is increasing interest in arm based devices so we anticipate this will only become more common. > > I'm happy to raise some PRs to address these issues. Given this is a pretty serious blocker, would there be someone available to review my work as I raise PRs? > > **Just spitballing some ideas:** > > I was thinking of initially introducing an env variable (e.g. `ELM_BINARY_LOCATION=/usr/local/bin/elm`) which, if set, will skip the download and platform verification. > > This would allow consumers who are on `linux-arm64` devices and using a translation layer like Box64 or Rosetta2 to use the `amd64` version while being aware of the limitations. It's also an extensible solution given it might work for `win-arm64` and other platforms with translation layers and without prescribed binaries. > > Secondly, I was thinking of adding some GitHub actions to automate the compilation of binaries for the various platforms and produce a GitHub release with those binaries. Supporting Linux ARM solves their immediate problem, so the discussion becomes more about a hypothetical future scenario which could happen as a new currently unsupported platform becomes popular. What happens then is that `npm install` fails due to the `elm` npm package erroring by design due to the unsupported platform. This fails the entire `npm install`, not just the install of the `elm` package. In Parcel’s case, it means contributors can’t install the project even if they didn’t plan to work on the Elm parts of Parcel. Here are some workarounds: - `npm install --ignore-scripts`. That will skip our install script, so no failure there. (If you then try to run `npx elm`, it’ll fail with the same message as the install script.) However, if you’re unlucky there’s some other dependency that does require install scripts. - `npm install --legacy-peer-deps` can help if `elm` is a peer dependency. For example, [elm-webpack-loader has `elm` as a peer dependency](https://github.com/elm-community/elm-webpack-loader/blob/a597c3f06711324318a2247a394001fdad4f1270/package.json#L36). That will skip automatic installation of “peer dependencies”, which will then skip installing the `elm` npm package. However, there might be other peer dependencies of the project that you now need to find and install yourself. Also, this only works if elm is actually specified as a peer dependency. If the project had been using [elm-land](https://elm.land/) instead of webpack, you’re out of luck: It has a [hard dependency on the elm npm package](https://github.com/elm-land/elm-land/blob/bad0e43140cd1da61b17344725d2c6429ebd4aad/projects/cli/package.json#L39). Here are some ideas we discussed for handling this situation: - Allow disabling the install script via an env var. - Allow also pointing to your elm binary via that env var. - Have `npm install` succeed on unsupported platforms, but `npx elm` (executing the installed elm “binary”) fail with the current error message. - Remove the install script completely. The first run will do the platform check and optimization for Linux and macOS anyway. - Parcel could release their own npm package, like `@parcel/elm`. Note: The only need to replace the main `elm` npm package, and can still re-use the binary packages (like `@evancz/elm_darwin_arm64`). **Note:** This one is doable without any changes to the `elm` npm packages (unlike the above points). For people running `npm install elm` themselves it’s ideal if it fails during installation on unsupported platforms. They should know about the problem, have a nice experience, and the npm package should be as simple as possible. From the perspective of Parcel, they want it to fail silently, maybe only when someone tries to run elm at some point or whatever. It’s not clear those two use cases are incompatible within a single package. We decided we just want to keep it simple and not make any design changes like the env var business.
github-actions[bot] commented 1 year ago

Thanks for suggesting these code changes. To set expectations:

Finally, please be patient with the core team. They are trying their best with limited resources.

ryan-haskell commented 1 year ago

Hey @lydell 👋

I've been testing out the @lydell/elm NPM package for the Elm 0.18.0 release, and noticed that GitHub action builds started failing recently:

image

The strange part is that this started happening about a week ago!

Those passing builds from 1 month ago were also using @lydell/elm (version 0.19.1-9), and they successfully installed. 🤯

Here's the error that started getting reported last week:

Run npm install

npm ERR! code 1
npm ERR! path /home/runner/work/elm-land/elm-land/projects/cli/node_modules/@lydell/elm
npm ERR! command failed
npm ERR! command sh -c -- node install.js
npm ERR! -- ERROR -----------------------------------------------------------------------
npm ERR! 
npm ERR! I support your platform, but I could not find the binary package (@lydell/elm_linux_x64) for it!
npm ERR! 
npm ERR! This can happen if you use the "--omit=optional" (or "--no-optional") npm flag.
npm ERR! The "optionalDependencies" package.json feature is used by Elm to install the correct
npm ERR! binary executable for your current platform. Remove that flag to use Elm.
npm ERR! 
npm ERR! This can also happen if the "node_modules" folder was copied between two operating systems
npm ERR! that need different binaries - including "virtual" operating systems like Docker and WSL.
npm ERR! If so, try installing with npm rather than copying "node_modules".
npm ERR! 
npm ERR! NOTE: You can avoid npm entirely by downloading directly from:
npm ERR! https://github.com/elm/compiler/releases/tag/0.19.1
npm ERR! All this package does is distributing a file from there.
npm ERR! 
npm ERR! --------------------------------------------------------------------------------

Here are some links if you're curious about more details:

Could this be an issue with caching node_modules or something?

lydell commented 1 year ago

For others following along, me and @ryannhg talked about his installation problem on Slack instead of here.

I verified that installing @lydell/elm works on all platforms on GitHub Actions here: https://github.com/lydell/elm-github-actions-test/actions/runs/3722282755

cyberglot commented 1 year ago

Any updates on this?

lydell commented 1 year ago

@cyberglot No, the status update at the top is still the latest news!

alshdavid commented 9 months ago

Does that mean the elm package is now officially unmaintained and development has moved to @lydell/elm?

supermario commented 9 months ago

@alshdavid no it does not mean that, it just means that the community has worked together to provide an unofficial binary for the missing platform until Evan has the time to prioritise this.

For example Evan committed support for the M1 arm binaries last week: https://github.com/elm/compiler/commit/ad9d9d42c4f623c2903ecf35daefaff011075977, though the npm package has not yet been updated there has been no official indication that Evan intends to no longer maintain the npm package.