honor magic comments

[dalimian]

@faceyspacey

Thanks alot for great plugin, and not just this one but the group of plugins you created.

Used them, they worked, saved me so much time

feedback for this plugin specifically though.

looks like it ignores webpack's magic comments and instead provides it's own way to generate the chunk names. creative, but potential issues I see with that are

1. webpack's magic comments is an official feature that many have come to use and depend on.
2. Potential source of confusion when a plugin disables a documented feature of the core library
3. I think there is a potential of conflicts in your algorithm,
   import('./components/componenta') in /site/page1/index.js, and
   import('./components/componenta') in /site/page2/index.js,
   will produce the same chunk path I believe

I would recommend to honor webpack's magic comments, or more so, require it. if you still like keep your alternative then make it an opt in feature, or only when magic comment is not specified.

[faceyspacey]

Glad you like the goods. There is a half completed PR someone did for allowing for Magic comment overrides. The completion of it would be welcome. It's in the universal import repo.

If you override it, then you have to provide the resolve option manually. Though maybe we could interpret it based on the comment.

[dalimian]

the most minimalistic and simple sol is to drop support for existing way in favor of magic comments, so basically require magic comments. In my project every dynamic import had a magic comment already so I took that direction.

this was the change https://www.screencast.com/t/NSEvc1rtR2b

would you want this repo to take that direction as well or you like to support both

[faceyspacey]

support both is the direction of this repo, as without it requires specifying not just the comment, but the resolveOption, which makes it less accessible.

That said various issues around the automation make it less accessible as well. It's a tough one.

Redux-First Router is going to start being able to flush chunk names at the route level. That said, it will be annoying there not to know the exact chunk name if it's automatically generated. So the minimalist direction would help there. Ultimately the way I think these React apps should be built is with many things moved to the "route level," and that might end up driving the direction of Universal, given otherwise, Universal is pushing a "component level" approach which is deemed less helpful overall than the approach RFR is promoting.

Either way, this repo should support both, and the way we do it is in a best of all worlds approach where we generate the resolve option based on the magic comment chunk name if no resolve option is provided. If you provide both, you're in control, sorta like "managed" vs "un-managed" <input /> components.

[dalimian]

i think i can create a pull request

as without it requires specifying not just the comment, but the resolveOption
not sure what is meant by resolveOption

can you clarify requirements, more detailed the better, what do u mean by resolve option.

just to clarify, out of your packages im only using this plugin and https://www.npmjs.com/package/extract-css-chunks-webpack-plugin, so might not familiar with other lingo.

[faceyspacey]

I actually meant the chunkName option. So if you customized the magic comment, you need to customize the chunkName option (which otherwise is automatically generated for you by this plugin):

universal(props => import(`./${props.page}`, {
  chunkName: props => props.page
})

So if you're gonna honor magic comments, it also requires you provide a matching chunkName:

universal(props => import(/* webpackChunkName: "custom/[request]" */`./${props.page}`, {
  chunkName: props => `custom${props.page}`
})

So to do the PR you're proposing truly correctly requires creating that option, and honoring tokens such as [request].

That said there already is a PR on the main universal-import package to do this:

faceyspacey/babel-plugin-universal-import#13

it's not completed. I totally forgot about it. I just left a comment about it. Basically it's mostly done, minus support for [request] and an issue regarding SSR. That PR is basically the perfect instructions for how to get this done and complete it. Let me know if you have any other questions.

[dalimian]

had to switch to a different project, won't have time to do this for now

[isidrok]

Hi, as I saw babel-plugin-universal-import already allows to define chunk names using magic comments.
Will that behaviour be added to this plugin aswell or should we stick with the other one?

I'm only using extract-css-chunks-webpack-plugin at the moment so don't really want to get too much tied to react-universal-component.

Awesome work btw!