gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
17.99k stars 750 forks source link

Target support not checked for unused unqualified imported values #3371

Closed joshi-monster closed 2 months ago

joshi-monster commented 4 months ago

Example:

import gleam/erlang.{rescue}
import gleam/io

pub fn main() {
    io.println("Hello, Joe!")
}

Generates an import in the JS module to gleam/erlang.{rescue}, even though this value is never used:

$ gleam run --target javascript
  ┌─ /home/arkan/Projects/probe/gleam/app/src/app.gleam:1:22
  │
1 │ import gleam/erlang.{rescue}
  │                      ^^^^^^ This imported value is never used

Hint: You can safely remove it.
   Compiled in 0.04s
    Running app.main
file:///home/arkan/Projects/probe/gleam/app/build/dev/javascript/app/app.mjs:2
import { rescue } from "../gleam_erlang/gleam/erlang.mjs";
         ^^^^^^
SyntaxError: The requested module '../gleam_erlang/gleam/erlang.mjs' does not provide an export named 'rescue'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:134:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:217:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:323:24)
    at async loadESM (node:internal/process/esm_loader:28:7)
    at async handleMainPromise (node:internal/modules/run_main:113:12)

Node.js v20.12.2

$ gleam -V
gleam 1.2.1

I noticed this in practice when trying to compile Wisp to Javascript. Wisp depends on Mist for mist_handler, which in turn imports gleam/erlang.{rescue} unqualified. I never called erlang-specific functions; instead I wanted to experiment whether or not you could use Wisp as a framework for serverless functions, since its Request -> Response structure is really similar to what you are expected to expose from Javascript.

~joshi :purple_heart:

lpil commented 4 months ago

Oh very strange! I wonder what is happening here. That function shouldn't be compiled at all.

Thank you

joshi-monster commented 4 months ago

The function itself (rescue) isn't compiled (which leads to the javascript error), but the import is generated regardless. The file erlang.mjs exists because it contains some custom type definitions.

I've looked briefly into this yesterday; the Javascript backend transpiles the imports directly, staying as close to the gleam source as possible - I'm guessing to not have to keep a mapping around, and possibly make the generated js more readable?

An easy fix might be to generate this javascript instead:

import * as $generated_name from './erlang.mjs';
const { rescue } = $generated_name; // doesn't throw, rescue here is just undefined
lpil commented 4 months ago

As in any function that is one target only shouldn't be generated for the other target. Here Wisp code seems to be generated for JavaScript?

joshi-monster commented 4 months ago

Yeah, I'm trying to compile wisp to javascript, that is on purpose. My goal was to see how far you could go, and potentially write a js_handler function that adapts wisp to Deno.serve/Vercel/Netlify serverless functions. Basically this is all for a "tired of webdev/javascript? you can literally just use gleam for everything" post.

This code already triggers the bug using wisp:

import gleam/io
import wisp
pub fn main() { io.debug(wisp.ok()) }

rescue is not generated, and that would be fine. But the import on the (non-)usage side still is, even though that function doesn't exist and is never used.

Compiling also succeeds, but trying to run the javascript throws a SyntaxError, because trying to import a symbol that is not exported is defined to do that (spec - step 7,c,ii)


I noticed that mist is not used in the generated Javascript, except for in one internal recursive function, maybe that causes mist to be included in the first place?

    // this is wisp.mjs
    // $mist is used exactly once, in a recursive function that is not used outside of its recursive call:
    import * as $mist from "../mist/mist.mjs";
    // wrap_mist_chunk is never used outside of itself
    function wrap_mist_chunk(chunk) {
      let _pipe = chunk;
      let _pipe$1 = $result.nil_error(_pipe);
      return $result.map(
        _pipe$1,
        (chunk) => {
          if (chunk instanceof $mist.Done) {
            return new ReadingFinished();
          } else {
            let data = chunk.data;
            let consume = chunk.consume;
            return new Chunk(
              data,
              (size) => { return wrap_mist_chunk(consume(size)); },
            );
          }
        },
      );
    }
lpil commented 3 months ago

Ah! I understand the problem now! The imported value isn't supported for the target, but we do target support checking on use, not on import, so if the import is unused it never gets analysed for support.

Yeah, I'm trying to compile wisp to javascript, that is on purpose. My goal was to see how far you could go, and potentially write a js_handler function that adapts wisp to Deno.serve/Vercel/Netlify serverless functions. Basically this is all for a "tired of webdev/javascript? you can literally just use gleam for everything" post.

It's not possible due to the two targets having completely incompatible concurrency and IO systems. You could at best make a different library with functions of the same names but with different types.

There's also the existing Glen framework which has a similar interface and is inspired by Wisp. https://github.com/MystPi/glen