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: mis-detecting imports on JavaScript when there is no checkJs #4040

Merged
merged 3 commits into from
Feb 20, 2020

Conversation

kitsonk
Copy link
Contributor

@kitsonk kitsonk commented Feb 19, 2020

This PR fixes an issue where we recursively analysed imports on plain JS files in the compiler irrespective of "checkJs" being true. This caused problems where when analysing the imports of those files, we would mistake some import like structures (AMD/CommonJS) as dependencies and try to resolve the "modules" even though the compiler would not actually look at those files.

@kitsonk kitsonk changed the title Fix mis-detecting imports on JavaScript when there is no checkJs fix: mis-detecting imports on JavaScript when there is no checkJs Feb 20, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

@ry ry merged commit 6431622 into denoland:master Feb 20, 2020
kitsonk added a commit to kitsonk/deno that referenced this pull request Mar 16, 2020
Refs denoland#4184
Refs denoland#4040

In denoland#4040 we changed the way JavaScript dependencies are analysed in the
TypeScript compiler.  When we encounter a JavaScript file that doesn't
have any types, and `checkJs` is disabled, we stop the analysis.  This
caused situations where when there was a typed file, loading an
untyped file, loading a typed file, we stop the analysis.  We didn't
specifically check this "chaining" behaviour in our tests, and there
were situations in the while where this behaviour was a regression, so
this introduces test to ensure the behaviour as designed is preserved.
kitsonk added a commit to kitsonk/deno that referenced this pull request Mar 17, 2020
Refs denoland#4184
Refs denoland#4040

In denoland#4040 we changed the way JavaScript dependencies are analysed in the
TypeScript compiler.  When we encounter a JavaScript file that doesn't
have any types, and `checkJs` is disabled, we stop the analysis.  This
caused situations where when there was a typed file, loading an
untyped file, loading a typed file, we stop the analysis.  We didn't
specifically check this "chaining" behaviour in our tests, and there
were situations in the while where this behaviour was a regression, so
this introduces test to ensure the behaviour as designed is preserved.
kitsonk added a commit to kitsonk/deno that referenced this pull request Mar 25, 2020
Refs denoland#4184
Refs denoland#4040

In denoland#4040 we changed the way JavaScript dependencies are analysed in the
TypeScript compiler.  When we encounter a JavaScript file that doesn't
have any types, and `checkJs` is disabled, we stop the analysis.  This
caused situations where when there was a typed file, loading an
untyped file, loading a typed file, we stop the analysis.  We didn't
specifically check this "chaining" behaviour in our tests, and there
were situations in the while where this behaviour was a regression, so
this introduces test to ensure the behaviour as designed is preserved.
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.

2 participants