Skip to content
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

Modules can be included twice resulting in subtle hard-to-debug bugs #464

Closed
matrixbot opened this issue Dec 2, 2015 · 5 comments
Closed
Labels
P2 T-Defect T-Task Tasks for the team like planning

Comments

@matrixbot
Copy link

Created by @ kegan:matrix.org.

This happens because webpack isn't smart enough to work out the absolute path of files when symlinks are involved. Therefore it sees vector-web/node_modules/matrix-react-sdk/foo as different to matrix-react-sdk/foo

@kegsay kegsay added T-Defect T-Task Tasks for the team like planning P2 labels Dec 2, 2015
@dbkr
Copy link
Member

dbkr commented Dec 2, 2015

Hopefully this only impacts us in development because of npm link symlinks, but if not it would be inflating the js size too which is annoying.

@kegsay
Copy link
Contributor

kegsay commented Dec 2, 2015

This can result in all sorts of fun. This bug was made because it turns out I managed to break the X on the desktop notifications toolbar because one require() was setting "hide the toolbar" and another require() was checking "is the toolbar hidden", and they were not hitting the same singleton.

@dbkr
Copy link
Member

dbkr commented Dec 2, 2015

Relevant webpack bug (implies it should now "support symlinks" although I think this means it doesn't include the file twice but does make them two separate instances of the class) webpack/webpack#554

@kegsay
Copy link
Contributor

kegsay commented Dec 2, 2015

Stopgap for now is to assign module.exports to a known singleton via globals.. I don't like this because it's very easy to forget to do this and it pollutes the window. namespace. I would prefer to do this exactly once if we have to, and then tie all the other singletons off that (which then come to think of it is starting to look and smell a lot like the Context singleton object I was proposing to inject into components, which also allows multiple instances in the same window to work).

@t3chguy
Copy link
Member

t3chguy commented Sep 9, 2020

This looks to be fixed with modern Webpack, analysing our bundles it looks sane

@t3chguy t3chguy closed this as completed Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 T-Defect T-Task Tasks for the team like planning
Projects
None yet
Development

No branches or pull requests

4 participants