Phoenix.js doesn't work with snowpack

ohmree commented 3 years ago

Environment

Elixir version (elixir -v): Erlang/OTP 23, Elixir 1.11.0
Phoenix version (mix deps): 1.5.7
NodeJS version (node -v): v15.6.0
NPM version (npm -v): 6.14.11 (note that I use yarn version 1.22.10 and not npm)
Operating system: Arch Linux

Expected behavior

After setting Phoenix up to use yarn and snowpack, Phoenix's named imports (Socket, LiveSocket) should work as expected when developing with snowpack.

Actual behavior

The browser can't find the imports - chromium's error message (which is a bit more specific than firefox's) is:
Uncaught SyntaxError: The requested module '../_snowpack/pkg/phoenix.js' does not provide an export named 'Socket'

I have a hypothesis as to why this is happening: since the main entry in Phoenix's package.json points to a webpack-bundled version, I think whatever webpack's doing to the raw imports is interfering with snowpack (or esbuild)'s ability to detect the imports and rewrite them to use its static asset path (by default it's /_snowpack/pkg).

Here are a few solutions I could think of:

Have the package.json main field point to the non-bundled version in assets/js, since the default Phoenix generator already comes with a webpack config that (I assume) bundles the dependencies automagically.
I don't know about compatibility with this one.
- Have a separate phoenix-es package with just the raw ES module, I think some other projects do this but I assume this adds more maintenance burden since (to the best of my knowledge) there isn't an easy way to have multiple packages in one package.json (I'm thinking of something like how Arch lets you create multiple related packages from a single build script).
- Have a default export of everything in addition to the named exports - this should :tm: work with snowpack, which explicitly supports only default imports for cjs modules (technically phoenix.js isn't a cjs module, but maybe it counts as one after the transformations made by webpack).
  This way is the easiest to implement upstream but it requires some minor modifications to one's app.js when using snowpack (or possibly other bundlers, I haven't tried any) which result in slightly uglier code IMO (again, this is a minor thing).
Here is a my snowpack config, it might be relevant to this issue and/or useful to anyone who wants to set up snowpack themselves.
Currently it works on priv/static in-place, which means there's a very short period after every asset modification when there aren't any assets because they were deleted and are being rewritten.
I've tried setting it up to work with a copy plugin for rollup but couldn't get it to use a separate directory for processing and then copy the result to priv/static.

P.S: while for my use case this issue isn't relevant to the phoenix_live_view package (since it default-exports LiveSocket and I only use that) it might still be worthwhile to allow users of other bundlers (or modern browsers) to use the other exports from said package.

Edit: I realized that I forgot to include a few other bits of config that are required for replicating my setup so I added them to the gist.

dbernheisel commented 3 years ago

I encountered this with Vite, and what fixed it for Phoenix and LiveView was specifying the "module" key in package.json with the value ./assets/js/phoenix.js file since it's written as ES modules. Likewise, LiveView's package.json needed the same adjustment and also needed to specify the morphdom dependency. Vite (and I think Snowpack and Rollup) will prefer this key since they are looking for ES modules. Other systems would continue looking at "main" which is the bundled version.

Maybe a better long-term solution is to consolidate Phoenix's assets/package.json into the root package.json.

Phoenix: https://github.com/dbernheisel/phoenix/commit/9dab39f7de0a517a44e35d3756c47a1a45037960 LiveView: https://github.com/dbernheisel/phoenix_live_view/commit/757d1676b8cbbe34566781285f14e115412749b6

I am planning on submitting a PR but I wanted to get into a better place with Vite to find if there are any other modifications needed for this type of JS build system.

But if that doesn't work out for some reason, you could try something like this without the adjustments above:

import * as Phoenix from "phoenix";
const { Socket } = Phoenix;
import * as LiveView from "phoenix_live_view";
const { LiveSocket } = LiveVIew;

EDIT -- added clarity.

chrismccord commented 3 years ago

I am not at all familiar with snowpack, but I would be interested in a simple PR that fixes the issue while not harming existing es6 + global window exports. Closing in the meantime as snowpack support is not something we maintain ourselves but feel free to submit a PR. Thanks!

ohmree commented 3 years ago

@dbernheisel Huh, I didn't think of doing import * as name from "module".
While it didn't work for me, it did inspire me to find something that does work:

import phoenix from "phoenix"
const {Socket} = phoenix
import LiveSocket from "phoenix_live_view"

@chrismccord Fair enough, for now I'll use the solution I found while waiting for @dbernheisel to make a PR with his changes :smile:

dbernheisel commented 3 years ago

Should be resolved for Phoenix with #4191. LiveView still needs it tho

phoenixframework / phoenix