-
-
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
Watching doesn't work on top level imports #9
Comments
The issue doesn't seem to be related to the regex either :/. Interesting problem |
Not sure if what I'm doing is wrong but setting |
Interesting, I will try to reproduce it. Thank you for the report! Big up for the repro! |
I think the problem comes from your code in this module. Hot reloading works well when having modules in this file:
Hot reloading works well on my side when doing |
Sorry for the slow gif |
Sorry, I misread your message. What is your version of node, npm and what OS do you use? |
Ubuntu 18.04 |
I first ran into this issue under docker on |
Are you using yarn workspaces or |
Could reproduce it on Linux. Investigating. |
References:
Can't do much for now, I'll check again why I enabled the |
Nice. Thanks man, will keep an eye out and report back if I face issues with |
Yes please! I am investigating on my side as well, did not face any error so far, but I know I added it for a reason I can't remember >< |
I'm having similar issue on Ubuntu. I'm using yarn workspaces with the packages in the root of the monorepo. This is my only config: I have to restart next to see changes in my shared components. |
@NathanielHill Did you try to disable symlinks and see what happens? |
I thought that was disabled by default 🤔 next-transpile-modules/index.js Line 30 in ca563f4
|
Just for the heck of this, I changed that to @martpie Am I not understanding what you're asking? |
thank you, this is what I meant :) I will investigate more on this issue this week-end. A guy from Webpack asked to provide a reproduction of this bug. |
Well, it's pretty obvious why. If I'm importing Then it will (in my case) try to import So it doesn't get passed through babel and fails due to syntax errors. |
@martpie Do we know if this is working on other platforms (with symlinks = Sounds like maybe this is a webpack problem with linux file watching? |
I have not gotten watching working for any shared code (outside my next dir). |
I have the same issue on my Mac now - just seem to arrive recently when I upgraded nextJS also |
@iwarner I did not reproduce it on macOS, can you provide a repro? |
Ok just tested some stuff: NextJS 7.0.3 -- This works √ Next 7.0.0 -> 7.0.3 This does not work unfortunately The Ideal combination I know is |
Yes, but hot reloading probably does not work. Can you provide a repro so I can try to fix it? |
Sorry yes the hot reloading works with -- This works √ -- Unfortunately I cannot provide a Repo on this For clarity the hot reloading is linked directly to the watching right? |
@NathanielHill I have a monorepo, and I'm doing a |
Hello guys, I have a lot on my plate right now, but I will try to work on it, it's a bit hard to debug as my development machine is a Mac, but I'll do my best. I have a couple of ideas I would like to try. Having a reproduction repo would help a lot. |
I have this issue at my job, and I don't have time right now to make a repo I'm sorry :-( |
Hi! @martpie I think you can use official with-yarn-workspaces example from next.js repo. In this case Right now I have similar issue, but with TypeScript. If you're using After few trys I came with quite simple solution (no idea if it’s correct, but it works). My project
As you can see the biggest change is that I'm not providing package names but folder names. Hope it helps :) |
this |
Not sure if this is a 'solid' solution :) I think we need 3 repos:
Each repo must be checked on MacOS / Windows / Linux with two configurations: Yesterday I checked the solution from the previous comment on Windows. It worked without a problem. But I'm not sure about this part:
|
@RafalFilipek I think your problem is unrelated and described by the FAQ: https://github.com/martpie/next-transpile-modules#i-have-trouble-with-transpilation-and-flowtypescript |
@martpie I have |
Hello! I created a demo repository for my situation with yarn workspaces: Basically a "main app" with a shared lib. Not even a specific babel configuration. When I change the code in the lib, my app doesn't reload, I have to restart it to have the change. I hope it can help you, and maybe others! I'm on Ubuntu 18.04. |
This is still a problem for me, but only for files in the top level of the package, as others have mentioned. |
I will try to jump again on the problem and see if it's fixable or if we need to wait for Webpack 5. Any help is welcome |
It's a really weird bug, but personally it's not too big a of a problem, because I can just put the code in a I'm not sure the root cause. The regex looks okay to me (tested it on regex101.com with some inputs) but I'm not super familiar with how webpack regex matching works. The fact that the top-level is missing makes me think something is "eating" it, which makes me suspect the regex... but not sure |
It seems you want It seems like since the simple case is transpiling some random es6/esnext package from npm, the side effect of changing the webpack config doesn't need to be the norm. Whereas people who are working in more complicated repos would be more likely to understand why they might need to opt in to this behavior, especially when combined with a README entry along the lines of "if you're using this to transpile local packages in a workspace/etc, set the config option...". Either way though, a config flag for controlling this would be great as it would avoid having to add a random webpack config change into the main body of an app's webpack config. |
I will think about adding a configuration option to change this. I may have time to work on this this week-end :) Sidenode @merrywhether, if you are transpiling npm packages, what is the problem with watching? You're not changing those files yourself 🤔 |
Oh, we have a workspaces/lerna monorepo with lots of packages consumed by a few Next applications. Each package has its own build:watch capability that we use during local dev, so we only need this module for transpiling public modules that have es6 features for those apps whose support matrix forces es5-only. For the apps that needed such transpiling, setting We explicitly want the Next apps to use the output from the packages' own build process (though it's a mostly-identical babel setup to what the apps do) since we also publish these packages and want to ensure we're working against the same output as any other consumer. |
@martpie Thanks for adding the |
|
Yikes, I don't have that problem with TS |
Is there a difference between
EDIT: yes, Fast Refresh works with the default settings, I did not have to set |
I understand that watching seems to work because it works in the sample repo linked here #5 (comment)
The thing is that if you import from a top level file in the package i.e.
/shared/index.js
watching doesn't work on that file. It's really odd, that is the case with yarn workspaces and file linking.See masad-frost/monorepo-typescript-next-the-sane-way@a5d89d8...master updating the contents of
shared/index
(index can be anything by the way) doesn't trigger a recompilation, it has to be an import from a subfolder of the moduleThe text was updated successfully, but these errors were encountered: