Fix some packages imports and fix windows hot reload problems

[davidmartos96]

Fixes #88

Hello! I stumbled across the issue when importing certain packages. It turns out that if we provide the path of .dart_tool/package_config.json to the workspace isolate, the interpreter can correctly handle those imports.

In order to generate the package_config.json we run dart pub get in the workspace without piping the output to the interpreter.

I've also added a few E2E tests for package imports.

[welcome[bot]]

Hi! Thanks for opening this pull request! :smile:

[fzyzcjy]

Great job! I will review after the CI is green (given that ubuntu and macos passes, it may not be hard to fix windows - since dart is cross-platform and this lib does not utilize special properties of a platform)

[davidmartos96]

Great job! I will review after the CI is green (given that ubuntu and macos passes, it may not be hard to fix windows - since dart is cross-platform and this lib does not utilize special properties of a platform)

Great!

I'm not entirely sure why it's failing on Windows when running "dart pub get" on the workspace. :thinking:

[davidmartos96]

@fzyzcjy I think I encountered the issue in this PR, which was how the process was being executed (no /bin/sh on Windows), but unfortunately there seems to be another issue not related with the new code.

I'm trying to run the master branch tests on Windows and they are failing with the same error as the one in this job. It's like the evaluate code isn't consistent. It may return null, or repeat some previous values. Maybe a Hot reload issue?

Would it be possible for you to re-run the Github actions on master? Just to see how CI runs master as of today.

[fzyzcjy]

@davidmartos96 I see, that does look like a bug prior to this PR!

[fzyzcjy]

btw, do you have a windows machine? I only have mac currently, so it will not be very easy to debug what's going on in windows.

If we can somehow make a minimal reproducible sample, then we can create a bug at official Dart repository - since I guess this behavior of vm service can be a bug.

[davidmartos96]

@fzyzcjy I think I'm on the right track for the issue on master. It works on Dart 2.19.6 and it fails on 3.0 and later.

When passing force=true to reloadSources it works as expected. Alternatively, when adding a 1 second delay before reloading sources it also works as expected.

Then I found this: https://github.com/dart-lang/sdk/issues/51937 Could it be that there is an internal bug on Dart for Windows where the precision of the modified files is not correct?

We could use force=true in the meantime, it's not like interactive should be used on big projects where hot reload performance should matter. What do you think?

[davidmartos96]

Or maybe it doesn't even matter for the dart_interactive case. Does isolate's sources include external packages? If not, force true would only be affecting a few dart files

[fzyzcjy]

Good job in finding the bug!

Could it be that there is an internal bug on Dart for Windows where the precision of the modified files is not correct?

That sounds reasonable. A wild guess: can it be caused by, for example, precision is 1s instead of 1ms? (just wild guess, not searched anything)

Or maybe it doesn't even matter for the dart_interactive case. Does isolate's sources include external packages? If not, force true would only be affecting a few dart files

I am not sure. Users can freely import anything, so it may include?

[davidmartos96]

@fzyzcjy I've just filed a separate issue in the dart/sdk repo if you want to include it in the comments https://github.com/dart-lang/sdk/issues/53546