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

Use a relative path for loading the JS main #566

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

marzipankaiser
Copy link
Contributor

Currently, the generated script for loading the compiled JavaScript uses an absolute path.
This prevents moving the generated files.

This makes it so a relative path is used instead.

@jiribenes
Copy link
Contributor

Motivation: If you compile some Effekt file like main.effekt on the JS backend using effekt --build --backend js main.effekt, the resulting file out/main is just a thin wrapper containing the following:

#!/usr/bin/env node
require('/a/long/path/to/./out/main.js').main()

Unfortunately, this prevents any attempts at packaging the resulting application in a reasonable way since the path is absolute.
Instead, it should perhaps be a relative path like require('./main.js').main() and we then trust the packager to bundle the runner script together with the JS file.

@jiribenes
Copy link
Contributor

Currently errors out on Windows with:

Error: Cannot find module './D:\a\effekt\effekt\out\tests\effekt.javascripttests\sideeffects.js'
Require stack:
- D:\a\effekt\effekt\out\tests\effekt.javascripttests\sideeffects__main.js
  [...]
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    'D:\\a\\effekt\\effekt\\out\\tests\\effekt.javascripttests\\sideeffects__main.js'
  ]
}

Copy link
Contributor

@jiribenes jiribenes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@marzipankaiser marzipankaiser merged commit 06dc86e into master Sep 2, 2024
2 checks passed
@marzipankaiser marzipankaiser deleted the feature/js-relative-import branch September 2, 2024 15:06
@jiribenes
Copy link
Contributor

We also might want to check what happens on the Chez backends wrt this issue 👀

@jiribenes
Copy link
Contributor

/**
* Creates an executable bash script besides the given `.ss` file ([[path]])
* and returns the resulting absolute path.
*/
def build(path: String)(using C: Context): String =
val out = C.config.outputPath().getAbsolutePath
val schemeFilePath = (out / path).canonicalPath.escape
val exeScriptPath = schemeFilePath.stripSuffix(s".$extension")
createScript(exeScriptPath, "scheme", "--script", schemeFilePath)
}

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.

2 participants