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
Create development bundle with inline source maps #159
base: main
Are you sure you want to change the base?
Conversation
Is this building only app.dev.js with “mix dev” or both files? |
Only app.dev.js. It builds with the same settings as |
I am worried we (the maintainers) will forget to run |
It can be done on the npm side with prebuild/prewatch scripts (and on the mix side with publish alias) |
Could we start two watchers on the |
If webpack build cache is race conditions free ;) |
I see, unfortunately I am worried about having two entry points/commands to run and having them getting out of sync. That would be a deal breaker for me. :/ Another option is for us to generate the source maps file separately and then, when sending the JS code to the client, we do a |
Thanks for taking a look at this! Just a quick question - why not just completely skip all optimization/minimizing when building the |
Not exactly. The source is transpiled by babel and when it's bundled by webpack it looses original module structure. |
I tried that and it seems to work |
js_path = Path.join(__DIR__, "../../../priv/static/js/app.js") | ||
css_path = Path.join(__DIR__, "../../../priv/static/css/app.css") | ||
js_bundle = if Mix.env() == "prod", do: "app.js", else: "app.dev.js" | ||
css_bundle = if Mix.env() == "prod", do: "app.css", else: "app.dev.css" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, this means we will only get source map when running in development locally. If that's the case, maybe we should generate the dev bundle in a tmp directory so we don't include it in the package? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To give a bit more context, packages always run Mix.env() == "prod"
. If the goal is only for local development, then I think the approach in #158 is simpler but I believe you also want source maps when used as a dep. Unfortunately though, there is no way to achieve it besides making it opt-in somehow.
Ping! |
@josevalim I got impression that there is no consensus on how and if to approach this. Besides even with the changes from that branch I'm not able do debug #165 as live_view and phoenix js deps come already minified and obfuscated. The better solution would be to get them unminified from npm and use js build tools in the release process. |
@lukaszsamson Both Phoenix & Phoenix Live View are going to expose Source Maps starting from the next release.
One small question about this PR: Why would you only add Source Maps to the dev bundle and not to the Prod one as well? |
Great
@maennchen IIRC there were concerns about bundle size - inline source maps significantly increase download size and parsing time on index.html |
Edit: never mind, we're loading it inline... |
This PR is a PoC of a solution mentioned in #158 (comment).
While it works, it's not optimal for local development (
mix dev
) because it performs unnecessary optimizations and source map generation. We would require another env variable to differentiate local development from running live_dashboard as a dependancy in dev and passing--mode development
indev.exs
.Resolves #150