-
Notifications
You must be signed in to change notification settings - Fork 85
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
fix: Use fully qualified module names #21
fix: Use fully qualified module names #21
Conversation
Thanks for the PR @jeremyckahn. I'm a little confused as to why this change is necessary on Trystero's side and what upstream is throwing that error. Is this a Webpack-specific issue? I'm hesitant to make this change since it would imply all my other JS projects will need to update all of their import statement to fully qualified, but I haven't seen any guidance on that being a new standard. Do you have any more info? |
Yes. This is the Webpack configuration rule that is responsible for this behavior: https://webpack.js.org/configuration/module/#resolvefullyspecified. I've seen this issue reported in a number of other places, such as:
In general, the recommended solutions seem to involve tweaking the consuming project's Webpack config as such: {
test: /\.m?js/,
resolve: {
fullySpecified: false,
},
} However, in an un-ejected Create React App 5 project, that doesn't seem possible to achieve at the moment because the Webpack configuration is abstracted away. Short of ejecting from Create React app and making the suggested Webpack configuration change, the best solution seems to be making changes to upstream projects like the one in this PR. It is frustrating that Webpack and Create React App collectively made this breaking change, but I'm not sure what a better solution for Create React App projects would be. |
@dmotz what do you think about the solution that's offered here? Is there anything you'd like to see changed before it can be merged? |
In case it's helpful, the issue that this PR fixes is now manifesting in my other Create React App-based project that uses Trystero. Here's a recent example of a breakage introduced by an automated Dependabot version update:
It seems that others who use Trystero with Create React App will likely experience this issue unless some kind of solution is implemented. |
Sorry for the wait on this, I've been busy. I merged this and published a new version (0.11.2) to npm. This still looks like a CRA problem to me caused by a Webpack strictness quirk, but I'm curious if fully qualified import paths are expected for all frontend libraries going forward. |
No worries! I appreciate you following up with a merge and release. I agree that this is something that CRA should fix by addressing facebook/create-react-app#11865, but until then downstream PRs like this seem like the most sensible workaround.
Me too. I haven't seen documentation either way regarding this. |
This PR fixes an issue that manifests in projects that use newer versions of Webpack:
A similar issue is described in anza-xyz/wallet-adapter#200, but manifests in Trystero as well. Among other things, this change will make Trystero compatible with Create React App version 5 (the latest version).
I also added a
.gitignore
so thatnode_modules
is not tracked by Git.