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

fix: stop type checking during runtime #22854

Merged

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Mar 12, 2024

In addition to the reasons for this outlined by @nayeemrmn in #14877 (which I think are reasons alone to not do this), this simplifies things a lot because then we don't need to implement the following:

  1. Need to handle a JSR module dynamically importing a module within it.
  2. Need to handle importing an export of a JSR dep then another export dynamically loaded later.

Additionally, people should be running deno check dynamic_import.ts instead of relying on this behaviour.

Landing this as a fix because it's blocking people in some scenarios and the current behaviour is broken (I didn't even have to change any tests to remove this, which is bad).

Closes #22852
Closes #14877
Closes #22580

@0f-0b
Copy link
Contributor

0f-0b commented Mar 12, 2024

Does this fix #22580 as well?

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I agree this is a better behavior.

@dsherret
Copy link
Member Author

Does this fix #22580 as well?

@0f-0b yes, I will add that. Thanks!

@dsherret dsherret changed the title fix: stop type checking unanlyzable dynamic imports at runtime fix: stop type checking during runtime Mar 13, 2024
@dsherret dsherret merged commit 43d066c into denoland:main Mar 13, 2024
17 checks passed
@dsherret dsherret deleted the fix_stop_type_checking_dynamic_imports branch March 13, 2024 20:38
nathanwhit pushed a commit that referenced this pull request Mar 14, 2024
In addition to the reasons for this outlined by @nayeemrmn in #14877
(which I think are reasons alone to not do this), this simplifies things
a lot because then we don't need to implement the following:

1. Need to handle a JSR module dynamically importing a module within it.
2. Need to handle importing an export of a JSR dep then another export
dynamically loaded later.

Additionally, people should be running `deno check dynamic_import.ts`
instead of relying on this behaviour.

Landing this as a fix because it's blocking people in some scenarios and
the current behaviour is broken (I didn't even have to change any tests
to remove this, which is bad).

Closes #22852
Closes #14877
Closes #22580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment