Cannot distinguish between two modules with same relative name
tomkelsey opened this issue · 6 comments
Expected Behavior
In folder1
:
import('./Content')
In folder2
:
import('./Content')
The two Content
components are different so should be seen as separate files despite having the same relative name.
Current Behavior
The two Content
components are seen as the same.
Possible Solution
Someone created a PR on react-loadable with a suggestion for a fix: jamiebuilds/react-loadable#111
Steps to Reproduce
As above under Expected Behaviour
Other Comments
Thanks for the quick resolution of my other issue - apologies for creating another so soon after!
Thanks for reporting @tomkelsey!
Understanding the issue
This issue happens because react-loadable
uses the chunk origin path
(file import path) as key, and as you can see, this is not a unique key
, which means that if you are importing two distinct files but with the same path (relative or absolute), it will generate this error.
Example:
Lets take a look in this folder structure:
source
├── components
│ ├── header
│ │ ├── index.js # Place where we will dynamically import the component bellow
│ │ │ ├── myComponentToBeDynamicallyImported.js
│ ├── menu
│ │ ├── index.js # Place where we will dynamically import the component bellow
│ │ │ ├── myComponentToBeDynamicallyImported.js
See that we have two components with the same name, myComponentToBeDynamicallyImported
, although it is in different folders, it can be dynamically imported with the same path
.
E.g.
import('./myComponentToBeDynamicImported')
By that, react-loadable
would get its path and use as key -> ./myComponentToBeDynamicallyImported
, which is the reason of this issue: duplicated key.
Sadly, the fix for it has to come from react-loadable
due to we have to use the same key as they to achieve the results from react-loadable-ssr-addon
.
Suggested fix
While react-loadable
won't fix this issue, the fix for this is quite simple, we just have to set a unique name
for the dynamically imported files.
In this case, you can customise its name using the following Webpack synthax:
import(/* webpackChunkName: "MenuDynamicImport" */ './myComponentToBeDynamicallyImported')
and
import(/* webpackChunkName: "HeaderDynamicImport" */ './myComponentToBeDynamicallyImported')
.
By which will generate two different files and as well, keys.
Note that you will need to configure Webpack settings as well to accept this:
module.exports = {
...
output: {
...
filename: '[name].bundle.js',
chunkFilename: '[name].chunk.js', // <- this is the required configuration
},
...
Also, you build output will have a batter readability!
For more informations, check the Webpack Code Splitting Documentation.
Thank you @themgoncalves for such an extensive reply and workaround. Much appreciated!
Just following up on your suggested fix: I believe it may cause an issue when you import the same component twice from different relative locations?
For example:
import(/* webpackChunkName: "SameComponent" */ './theSameComponent')
then elsewhere in the project:
import(/* webpackChunkName: "SameComponent" */ '../../theSameComponent')
In the generated assets file, I believe they'll only be one entry in origins
for './theSameComponent'
and not '../../theSameComponent'
And so when rendering on the server for '../../theSameComponent'
it looks up the key, cannot find it, and therefore does not return the required assets?
Just wondering if something like this might work:
getChunkOrigins(chunk) {
const { modules, id, names } = chunk;
const origins = new Set();
for (let i = 0; i < modules.length; i += 1) {
const { reasons } = modules[i];
for (let j = 0; j < reasons.length; j += 1) {
const { type, userRequest } = reasons[j];
if (type === 'import()') {
origins.add(userRequest);
}
}
}
if (origins.size === 0) {
return [names[0] || id];
}
return Array.from(origins);
}
getAssets(assetsChunk) {
for (let i = 0; i < assetsChunk.length; i += 1) {
const chunk = assetsChunk[i];
const { id, files, siblings = [], hash } = chunk;
const keys = this.getChunkOrigins(chunk);
keys.forEach(key => {
this.assetsByName.set(key, {
id,
files,
hash,
siblings
});
});
}
return this.assetsByName;
}
@tomkelsey I will have to do some investigation here around your report and get back to you later!
Thanks for your collaboration so far 🤟
@tomkelsey your solution worked like a charm! It was released in the v0.1.4
, by which you can update you package right now.
Have a nice day.