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

Type changes to symlinked module files in a monorepo are not updated after the initial launch #557

Closed
J-Rojas opened this issue Jan 6, 2021 · 26 comments
Labels
bug monorepo Related to project references, build, etc.

Comments

@J-Rojas
Copy link

J-Rojas commented Jan 6, 2021

Current behavior

Within a monorepo workspace with symlinked packages, changes to files in the symlinked modules are detected, however the types are not dynamically updated during a dev server session.

For example, the following code compiles correctly after the initial launch of webpack.

// original code somewhere in symlinked workspace package
export class Foo {
    bar() { return "bar" }
}

let foo = new Foo()
console.log(foo.bar())


// original code somewhere in main workspace

import { Foo } from 'monorepo-package'
new Foo().bar() // this works!

However incremental changes like the ones below will break.

// updated code somewhere in symlinked workspace package
export class Foo {
    bar() { return "bar" }
    baz() { return "baz" }
}

let foo = new Foo()
console.log(foo.bar())
console.log(foo.baz()) // this works! no errors!


// updated code somewhere in main workspace

import { Foo } from 'monorepo-package'
new Foo().bar() // this works!
new Foo().baz() // this fails! 'baz' is not detected as a new member function


Expected behavior

Changes to types should be updated dynamically across package boundaries in a monorepo.

Steps to reproduce the issue

See the above example.

Issue reproduction repository

N/A... code above might be enough for a repro.

Environment

  • fork-ts-checker-webpack-plugin: v6.0.alpha (latest master does not detect symlinks changes at all)
  • typescript: v4.1.3
  • eslint: v4.19.1
  • webpack: v4.44.1
  • os: Mac OS 10.14.6
@J-Rojas J-Rojas added the bug label Jan 6, 2021
@piotr-oles
Copy link
Collaborator

Could you send your webpack setup or create a reproduction repository? I tested symlinks on my local machine and it works for me, but probably it's due to a different configuration/use-case.

@J-Rojas
Copy link
Author

J-Rojas commented Jan 10, 2021

I discovered the way to fix this issue is to use the build: true option while using project references to generate the type definitions of the subprojects. Otherwise, the original type definitions are used, leading to stale errors.

// vue.config.js snippet
new ForkTsCheckerWebpackPlugin({
        typescript: {
          extensions: {
            vue: true
          },
          build: true
        }
      })

This fix does not appear to be consistent. I tried it in my reproduction repo and was not able to have it work 100% of the time. There could be some combination of default Vue CLI/Webpack behavior here (like caching) that might be at play.

@J-Rojas
Copy link
Author

J-Rojas commented Jan 10, 2021

Here's the repoduction repo.

https://github.com/J-Rojas/ts-vue-hello-world

Steps:

Install

yarn install

Switch to error branch.

git checkout error-inconsistent-build

Run the serve command. You should see an error for a missing baz definition.

yarn run serve

While running the serve command, switch to the new branch that should add the missing function.

git checkout add_function

The serve will update, but the function definition will not be picked up. If you kill the serve command and launch again, the error should be gone.

@lllllllqw
Copy link

I also encountered the same problem.

Environment

  • @babel/preset-typescript: v7.12.7
  • webpack: v4.46.0
  • os: Mac OS 10.15.7

@piotr-oles
Copy link
Collaborator

@J-Rojas thanks for the reproduction repository! I'm not sure when I will have some time for digging into this issue - I'm going on 2 week vacation in few days. So if not this week, then earliest is in 3 weeks

@renke
Copy link

renke commented Mar 14, 2021

I seem to have the same problem, I have shared library written in TypeScript which is build via watch and the output is placed in lib directory. The library is npm link'd into my Webpack application. When the lib directory is changed a rebuild of the app is triggered type changes (like a new function) are not picked up, the app itself can definitely use the function (like, I saw a the output of console.log used in that function) when you ignore the type checking errors.

@AlonMiz
Copy link

AlonMiz commented Mar 21, 2021

having the same issue.
build:true helps but in my case it doesn't take the last output. it needs another "kick" (changing some file) after the types.d.ts is built

in my case it even worse, as the package I'm trying to watch is already "built" and I'm exporting the index.js and not ts.
in order to overcome that I added path alias to tsconfig and webpack alias to trick webpack into taking the index.ts file

{
  "compilerOptions": {
    "paths": {
      "@pkg/enums": ["../packages/enums/src"],
    },
  },
  "references": [
    { "path": "../packages/enums/src" }
  ],
  "include": [
    "../packages/enums/src",
    "src/**/*"
  ]
}
const config = {
 resolve : {
    '@pkg/enums': path.join(__dirname, '../../', 'packages/enums/src'),
  }
}

@piotr-oles
Copy link
Collaborator

I was trying to fix that, but no progress so far :/ I can reproduce the issue, but I'm not sure why it behaves like this... I will try again in few days

@AlonMiz
Copy link

AlonMiz commented Mar 22, 2021

@piotr-oles Thanks. reproducing is 50% of the solution, so that's great news :)
if you need help. pls let us know

@piotr-oles piotr-oles added the monorepo Related to project references, build, etc. label Apr 21, 2021
@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 26, 2021
@J-Rojas
Copy link
Author

J-Rojas commented Jun 26, 2021

bump

@stale stale bot removed the wontfix label Jun 26, 2021
@Pantura
Copy link

Pantura commented Feb 4, 2022

Any luck with this?

@AlonMiz
Copy link

AlonMiz commented Feb 5, 2022

moved to vite . works great with monorepo and ts. it listens to changes in each imported package

@piotr-oles
Copy link
Collaborator

Could you test with the latest version? I found some bugs and fixed them :)

@Pantura
Copy link

Pantura commented Feb 7, 2022

Still not working:

"webpack": "5.68.0",
"typescript": "4.5.5",
"fork-ts-checker-webpack-plugin": "7.1.1",

ts-config has
"preserveSymlinks": true,

Project has symlinked node_module that points to ../../library. Webpack correctly builds the changes and these are available in the browser but type checking is always done to a version of dependency that existed when watch started.

@piotr-oles
Copy link
Collaborator

And your code doesn't use vue, right?

@aovchinn
Copy link

aovchinn commented Feb 7, 2022

I am looking at the same code base as @Pantura

vue is not used

I digged into source code of this plugin, found that on file change, fileWatcherCallbacks is undefined
but it seems like nobody calls .close on watcher, is there other ways to delete callback from fileWatcherCallbacks ?
looking at src/typescript/worker/lib/system.ts

initially callback is assigned properly

some debugging output:

awaited hooks.start.promise
debug Submitting the getIssuesWorker to the pool, iteration 2.
debug Running the getIssuesWorker, iteration 2.
debug Submitting the getDependenciesWorker to the pool, iteration 2.
debug Running the getDependenciesWorker, iteration 2.
invalidateCache
invalidateCache
getDependenciesWorker invalidate
IssuesWorker didDependenciesProbablyChanged

awaited getDependenciesWorker

change redacted/selectors.ts
invokeFileChanged redacted/selectors.ts
invokeFileWatchers redacted/selectors.ts
Booooooohoo for redacted/selectors.ts <--- no callback

@aovchinn
Copy link

aovchinn commented Feb 7, 2022

I missed obvious detail, that callback added with normalized path "app/node_modules/redacted/selector.ts" but later it is called with absolute path to the linked module "/root/src/redacted/selector.ts", and this is why there is no handler on change

@piotr-oles
Copy link
Collaborator

Ok, so we should use realpath here. The issue can be when we delete symlink - then we should invoke fileDeleted, but we will not be able to get realpath. Because of that, I think we would need to store some in-memory cache of mapping from linked modules to absolute paths.

@aovchinn
Copy link

I wonder if it's appropriate to always use realpath, I think webpack does it by default
controllable with resolve.symlinks (https://webpack.js.org/configuration/resolve/#resolvesymlinks)

@aovchinn
Copy link

if I use this realPath in createWatcher, typechecking works

function createWatcher<TCallback>(watchersMap: Map<string, TCallback[]>, path: string, callback: TCallback) {
  const normalizedPath = realFileSystem.realPath(path);

why initially watcher gets this "incorrect" path ? ("app/node_modules/redacted/selector.ts")
could it be something with webpack itself ? who creates a watcher ?

@aovchinn
Copy link

I've created a reproducible small example https://github.com/aovchinn/test-fork-ts-checker/tree/main
it seems it is due to silly tsconfig paths alias, module: [node_module/module]

@piotr-oles
Copy link
Collaborator

@aovchinn I created a PR that should fix that. It fixes the issue in the repo you created :) Thanks for your help!

@piotr-oles
Copy link
Collaborator

Could you check if the newest version works correctly in your case?

@aovchinn
Copy link

yes, it is working, thanks 👍

@piotr-oles
Copy link
Collaborator

I'm closing this issue then :)

ferm10n added a commit to ferm10n/fork-ts-checker-webpack-plugin that referenced this issue Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug monorepo Related to project references, build, etc.
Projects
None yet
Development

No branches or pull requests

7 participants