-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[gatsby-plugin-netlify-cms] switch to require.resolve
for modules hoisting
#19860
Conversation
This is a non-trivial change. Any chance you could add a test for it? Basically before it would be relative paths ( (There's a number of reasons why this change is non-trivial. For example, white/blacklists could be doing a prefix check, those would break with this change.) Is there an issue about this problem? Seems to me like this would affect many people. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Non-trivial change without tests)
cc @erquhart |
I don't quite understand what you mean and about white/blacklists could be doing a prefix check. But if you don't do this fix how do you resolve the problem I mentioned above? |
@rwu823 I definitely want to help to resolve that issue (can please you point me to the report, or file a new one in https://github.com/gatsbyjs/gatsby/issues/new?template=bug_report.md ?). I also want to make sure that the fix is not going to cause other problems and so I'm asking for a test :) |
This was just added a day ago, looks like the right move. @erezrokah should sign off on it. |
This looks much better. I'm good with it |
@pvdz I had recently changed the plugin code to copy the relevant dependencies instead of letting Webpack do the bundling. This fix makes sure the dependencies are resolved from the correct location. |
Holy buckets, @rwu823 — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! |
Description
This fix in mono-repo based project the
node_modules
hoist to project root.