AlexRiedler / my_project

Elixir Test Project
3 stars 1 forks source link

Thanks a lot for this #1

Open happysalada opened 6 years ago

happysalada commented 6 years ago

I'll have a good look at it this weekend and tell you if I see anything that can be improved.

happysalada commented 6 years ago

So I tried your setup.

document.addEventListener("DOMContentLoaded", initializeApp, false)


Last but not least, a big thank you for getting me started on this. I needed a push to figure this out and your repo was a lot of help!

Some unrelated stuff that you might be interested in:
A couple of comments
- why do you use alpine 3.6 explicitely in the dockerfile when elixir 1.7.2 is using alpine 3.8 ?
- apk update, just updates the package repository reference. I think you want to run `apk upgrade` to upgrade the packages.
- I don't think you want to have

CMD ["/app/bin/my_project", "foreground"] ENTRYPOINT ["/app/bin/my_project"]

in your docker file.
You would either want

ENTRYPOINT ["/app/bin/my_project"] CMD ["foreground"]

or just

CMD ["/app/bin/my_project", "foreground"]


The advantages of the latter being that you can run other scripts on your container
- be careful with the watcher you set (on phoenix master it uses webpack with `--watch-stdin`. If you don't have this, when you ctr-c you have node ghost processes persisting. (a detail, I'm just saying that because it took me a long time to find)

Some questions
- I've never had to run a cluster of nodes, is that why you define the EPMD and BEAM port, to connect nodes together?
- I haven't used edeliver. I didn't know you could use it with docker. Are you using edeliver to copy the image onto the server? Why not do a scp then run the image with docker-swarm? (just curious)
AlexRiedler commented 6 years ago

Webpack

I found that to use dynamic import there is another way (albeit, a little painful)...

you probably know this, but using an async await in the initialize function enables several dynamic imports.

Nope didn't know this at all, more of a backend developer then front-end. Good to know!

Docker Stuff

A lot of the docker stuff is an experiment; I actually have a branch (WIP) to the docker stuff that fixes a lot of the issues that you outlined above (particularly the entrypoint and cmd). See https://github.com/AlexRiedler/my_project/tree/docker-releases

The docker stuff was based off another repo example on edeliver/issues/121 ; some of which felt weird to me in general. edeliver currently doesn't support docker. However, you can build locally in docker to avoid having a dedicated build server, then just edeliver to staging and production.

I've never had to run a cluster of nodes, is that why you define the EPMD and BEAM port, to connect nodes together?

yeah this is so you can remotely connect to the erlang vm if necessary for both management and clustering

THANKS FOR THE WRITE UP

Glad I could be of service; and Thanks for Reaching out! I'll probably post about my findings above if I can avoid the issue.

happysalada commented 6 years ago

On the webpack stuff.

On the clustering stuff.

A couple of comments on the docker stuff

AlexRiedler commented 6 years ago

Webpack

Clustering

Docker

happysalada commented 6 years ago

Okay, I gave this a real go, but couldn't make it work.

I used

optimization: {
    splitChunks: {
      chunks: ({name}) => !name || !name.includes('admin'),
    }
  },

which is just a way to tell it to split chunks on all except the ones where the name includes "admin". I can see the chunks being split up

update  [emitted]         update
                                                                 1.chunk.js    61.8 KiB                                                                   1  [emitted]
                                                                 2.chunk.js    59.8 KiB

however they don't appear in the manifest, only my entrypoints name do. of course when I try to load the page, it doesn't display any error, but it doesn't show me the things that the js is supposed to load as well. looking at the doc of the webpack manifest plugin publicPath is supposed to be a string, so that's what I used (just "/") I couldn't find the merge option, so I didn't use it. Appart from that, my setup is very similar to yours. not really sure what is the problem.

AlexRiedler commented 6 years ago

You shouldn't need the chunks in the manifest because they will be dynamically loaded by the correct path inside the javascript that included them.

Although when I am testing here, it seems named dynamic chunks also get put into the manifest.json.

import(/* webpackChunkName: "dynamic" */ "./dynamic").then(({ default: dynamic }) => {
  dynamic();
});

I really don't understand chunk optimization stuff, I could never get it to work as I thought it would.

happysalada commented 6 years ago

yes correct. The limitation is that you can't name all of your chunks. The chunks that you name under imports are all the async chunks, if you want to name the split from the initial chunks, I don't know how to do it. The problem is then that webpack tries to load assets with a relative paths (when not on the default route) and doesn't find them. So I think the maximum that can be done here, is to split only async chunks, then you can probably use your webpack setup.

I would add to that as well, that to preserve caching, you have to include the setting to keep the runtime in a single chunk. https://webpack.js.org/guides/caching/