[jquery] Fix module import error in jquery 4.0.0#74984
Conversation
|
@TomiBelan Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through. This is a live comment that I will keep updated. 1 package in this PRCode ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 74984,
"author": "TomiBelan",
"headCommitOid": "4cc1f001ea8da6af7da160b4ee158bb5c77b2fe9",
"mergeBaseOid": "1d859fa5f1c4b759f25935c4e3257a63237a4efe",
"lastPushDate": "2026-05-11T21:14:23.000Z",
"lastActivityDate": "2026-05-12T06:21:46.000Z",
"hasMergeConflict": false,
"isFirstContribution": true,
"tooManyFiles": false,
"hugeChange": false,
"tooManyCommits": false,
"tooManyReviews": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "jquery",
"kind": "edit",
"files": [
{
"path": "types/jquery/index.d.mts",
"kind": "definition"
},
{
"path": "types/jquery/slim.d.mts",
"kind": "definition"
}
],
"owners": [
"leonard-thieu",
"borisyankov",
"choffmeister",
"Steve-Fenton",
"Diullei",
"tasoili",
"seanski",
"Guuz",
"ksummerlin",
"basarat",
"nwolverson",
"derekcicerone",
"AndrewGaspar",
"seikichi",
"benjaminjackman",
"JoshStrobl",
"johnnyreilly",
"DickvdBrink",
"King2500",
"terrymun",
"martin-badin",
"princefishthrower"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [],
"mainBotCommentID": 4425251938,
"ciResult": "fail",
"ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4cc1f001ea8da6af7da160b4ee158bb5c77b2fe9/checks?check_suite_id=68495588348"
} |
|
Hey @TomiBelan, 😒 Your PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Please consider adding tests to cover the change you're making. Including tests allows this PR to be merged by yourself and the owners of this module. This can potentially save days of time for you! |
|
🔔 @leonard-thieu @borisyankov @choffmeister @Steve-Fenton @Diullei @tasoili @seanski @Guuz @ksummerlin @basarat @nwolverson @derekcicerone @AndrewGaspar @seikichi @benjaminjackman @JoshStrobl @johnnyreilly @DickvdBrink @King2500 @terrymun @martin-badin @princefishthrower — please review this PR in the next few days. Be sure to explicitly select |
|
@TomiBelan The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! Note: builds that are failing do not end up on the list of PRs for the DT maintainers to review. |
That doesn't look related to this PR. Please review it anyway and ignore the bot. |
|
Just to check Chesterton's Fence before we proceed on this one... why were these lines originally added. I suspect we may resolve whatever problem you are experiencing, but open up problems previously fixed for other folks. Can we check the history for the line being removed so we understand the issue or PR that motivated it's addition? |
Please fill in this template.
pnpm test <package to test>.If changing an existing definition:
package.json.This is my attempt to fix the issue described in #74639 and #74367 (comment), which is that simply importing jquery gives an error if you use modern TypeScript options.
Please check my work. I don't really know what I'm doing. I thought somebody would fix it by now, but here we are.
This PR does not contain any regression tests, because dtslint only allows
"module": "commonjs"or"node16", in which the issue doesn't happen. It also doesn't allow"moduleResolution".Best I have is this manual test:
$ echo 'import $ from "jquery";' > main.ts(You may notice that with new defaults in TypeScript 6.0, the error is trivially reproducible with default tsc options.)
$ pnpm tsc main.ts(tsc prints nothing.)
This should prove that the PR fixes the issue. I hope it doesn't break anything else.
FYI CC @DreierF, @valipanahi, @gabritto, @andersk, @marekdedic