-
Notifications
You must be signed in to change notification settings - Fork 663
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
Compiled Elm code is not friendly to JS modules #1029
Comments
I think that this would be a very valuable approach to adopt. |
Seems reasonable. Can this be done as part of the larger codegen overhaul? |
Wow. I don't know what SES is, or why we need the noConflict function. |
A trimmed-down implementation like Bacon's seems fine. I'd say the main thing is that Node, CommonJS, AMD, and |
Can we close this down and open an issue on elm-make just outlining the smaller recommendation and linking here. When Joey and I get to this, we will use that snippet. |
Current JS best practices have abandoned the 1990s-era practice of slapping another
<script>
tag on the page, expecting that script to mutate the globalwindow
object, and moving on. Today, the best practice is to use a module system (e.g. RequireJS, AMD, ES6 modules) and a build tool (e.g. browserify, webpack) to combine them.At the moment, compiled Elm code supports only the 90s approach: add
Elm
to the global object and move on. As such, there is no way to (for example)require
an Elm module from within Node, short of an extra wrapping step. The best practice to avoid this is to add a snippet of boilerplate which detects the JS module environment and proceeds appropriately. Here is an example from a popular JS library.This would enable the following, in either Node or in the browser (when using something like Browserify):
...as opposed to the current "add it to the page and expect the Global to be there."
Concretely, this would mean adding the following boilerplate (adapted from the aforementioned Q snippet) to compiled Elm output:
This might seem like a lot, but:
require
multiple times. Each will be namespaced separately with no conflicts.The text was updated successfully, but these errors were encountered: