Skip to content

fix: handling of circular dependencies #181

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

Merged
merged 1 commit into from
Feb 28, 2025
Merged

fix: handling of circular dependencies #181

merged 1 commit into from
Feb 28, 2025

Conversation

mohd-akram
Copy link
Contributor

@mohd-akram mohd-akram commented Feb 25, 2025

Fixes #163

In the integration CI, this fixes importing the svelte package.

It does not result in identical behavior to Node.js, but this is very much a corner case:

// Importing 'a' first will fail in Node.js (with or without the loader)
await import('./test/fixtures/circular-a.mjs')
await import('./test/fixtures/circular-b.mjs')
// If you try and ignore the error
try { await import('./test/fixtures/circular-a.mjs') } catch { }
// it affects the subsequent import of 'b', which will now fail
await import('./test/fixtures/circular-b.mjs')
// If 'b' is imported first, it succeeds (as in the test case)
await import('./test/fixtures/circular-b.mjs')
// If the import of 'a' is done after, it also succeeds with { foo: 1, bar: 2 }
// With this PR, we return { foo: 1, bar: undefined }
await import('./test/fixtures/circular-a.mjs')
// Given that importing 'a' directly is fragile, it's likely
// not imported directly in real-world scenarios but rather through 'b'

@mohd-akram mohd-akram changed the title Fix handling of circular dependencies fix: handling of circular dependencies Feb 26, 2025
@timfish timfish merged commit b58092e into nodejs:main Feb 28, 2025
47 of 48 checks passed
@mohd-akram mohd-akram deleted the fix-cycle branch February 28, 2025 14:09
mydea added a commit to getsentry/sentry-javascript that referenced this pull request May 12, 2025
mydea added a commit to getsentry/sentry-javascript that referenced this pull request May 12, 2025
See
https://github.com/nodejs/import-in-the-middle/releases/tag/import-in-the-middle-v1.13.1

### Bug Fixes

* handling of circular dependencies
([#181](nodejs/import-in-the-middle#181))
([b58092e](nodejs/import-in-the-middle@b58092e))
* importing JSON files
([#182](nodejs/import-in-the-middle#182))
([8c52014](nodejs/import-in-the-middle@8c52014))
* warning from use of context.importAssertions
([#179](nodejs/import-in-the-middle#179))
([8e56cf1](nodejs/import-in-the-middle@8e56cf1))

This version is already allowed by our range, but in order to ensure
everybody gets this, bumping it here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when using import-in-the-middle with @langchain/core: ReferenceError: RunTree is not defined
3 participants