Prevents auto-import in `module: "none"` #55328

hardikkoul · 2023-08-10T10:23:06Z

andrewbranch

I looked a little closer, and I think updating compilerOptionsIndicateEs6Modules would be better. This also needs an accompanying test. Thanks!

…aceholders (microsoft#54188)

hardikkoul · 2023-08-11T09:34:39Z

I looked a little closer, and I think updating compilerOptionsIndicateEs6Modules would be better. This also needs an accompanying test. Thanks!

Got it, will update the method accordingly.

hardikkoul · 2023-08-11T13:49:24Z

Hey @andrewbranch . I've removed the check and applied changes to the method. I'm not so familiar with the Unit Test scenarios , can you guide me a bit there and i will complete that part as well. Also let me know if the changes are not as per your satisfaction.

andrewbranch · 2023-08-11T16:24:26Z

Take a look at some fourslash completions tests like https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/javaScriptModules18.ts

You can use // @Filename to set up two files, one with an export, one with a marker, and // @module: none.

…ft#55294)

…5181)

hardikkoul · 2023-08-13T07:05:06Z

Take a look at some fourslash completions tests like https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/javaScriptModules18.ts

You can use // @Filename to set up two files, one with an export, one with a marker, and // @module: none.

I have added a scratch Unit Test , please check and suggest changes accordingly. @andrewbranch

hardikkoul · 2023-08-13T07:08:11Z

@hardikkoul please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree

andrewbranch · 2023-08-14T17:20:45Z

tests/cases/fourslash/autoImportNonModules.ts

+//// /**/
+
+goTo.marker("");
+verify.completions();


Suggested change

verify.completions();

verify.completions({

excludes: ["f"],

preferences: {

includeCompletionsForModuleExports: true

}

});

andrewbranch · 2023-08-14T17:25:28Z

src/services/utilities.ts

+        // Prevent ES6 module indication for non-module files
+        if(compilerOptions.module === ModuleKind.None){
+            return false;
+        }


spacing

--module none does not mean files are “non-modules.” It honestly doesn’t mean anything at all.

Suggested change

// Prevent ES6 module indication for non-module files

if(compilerOptions.module === ModuleKind.None){

return false;

}

if (compilerOptions.module === ModuleKind.None) {

return false;

}

spacing

--module none does not mean files are “non-modules.” It honestly doesn’t mean anything at all.

On it

spacing

--module none does not mean files are “non-modules.” It honestly doesn’t mean anything at all.

Do you have any suggestions or heads up of where i can get the file module configurations from instead of compilerOptions?

I’m only objecting to the comment, not the implementation. A file is considered a module if it has sourceFile.externalModuleIndicator set. But we don’t actually want to prevent auto-imports into non-module files because you might have a file that’s not yet a module but you intend to make it a module. --module none I guess makes it an error to have a module, but the option is implemented inconsistently, and we’ve thought at times about changing what it means, and I was a little surprised to hear about anyone using it at all. Auto-imports generally tries to avoid creating errors, so I’m fine with this feature, but I don’t like the comment because it seems to imply some level of coherence and intentionality behind --module none, and I’m not convinced there is any.

I’m only objecting to the comment, not the implementation. A file is considered a module if it has sourceFile.externalModuleIndicator set. But we don’t actually want to prevent auto-imports into non-module files because you might have a file that’s not yet a module but you intend to make it a module. --module none I guess makes it an error to have a module, but the option is implemented inconsistently, and we’ve thought at times about changing what it means, and I was a little surprised to hear about anyone using it at all. Auto-imports generally tries to avoid creating errors, so I’m fine with this feature, but I don’t like the comment because it seems to imply some level of coherence and intentionality behind --module none, and I’m not convinced there is any.

ohh, my bad , i thought you were talking about the implementation.

removed the comment , you can check now @andrewbranch :)

…him (microsoft#55273)

Co-authored-by: Damien Engels <[email protected]>

…icrosoft#55298)

…oft#55360)

…r module resolution and type reference directive (microsoft#55376)

…crosoft#55381)

…otype method (microsoft#55513)

…ject (microsoft#55514)

…missing modules (microsoft#55519)

…osoft#55522)

microsoft#55279)

andrewbranch · 2023-08-28T17:54:52Z

src/services/utilities.ts

@@ -1840,6 +1840,9 @@ namespace ts {
        return program.getSourceFiles().some(s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s) && !!s.externalModuleIndicator);
    }
    export function compilerOptionsIndicateEs6Modules(compilerOptions: CompilerOptions): boolean {
+        if (compilerOptions.module === ModuleKind.None){


Suggested change

if (compilerOptions.module === ModuleKind.None){

if (compilerOptions.module === ModuleKind.None) {

andrewbranch · 2023-08-28T17:55:18Z

One missing space and then there are some merge conflicts that need resolving, but otherwise looks good!

… symbol (microsoft#55529)

…nts (microsoft#55472)

…oft#55282)

…ment position (microsoft#55552)

…xtends another (microsoft#55252)

…5424)

hardikkoul · 2023-08-29T04:36:46Z

One missing space and then there are some merge conflicts that need resolving, but otherwise looks good!

got it , on it now

…cript into bug55256-Hardik

typescript-bot · 2023-08-29T05:14:28Z

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

typescript-bot · 2023-08-29T05:14:29Z

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

Prevents auto-import in non-module files

a661017

typescript-bot assigned andrewbranch Aug 10, 2023

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 10, 2023

andrewbranch requested changes Aug 10, 2023

View reviewed changes

Andarist and others added 2 commits August 10, 2023 12:50

Allow intersections to be used as valid types for template literal pl…

28cd1fb

…aceholders (microsoft#54188)

changing "compilerOptionsIndicateEs6Modules"

6b1f9bf

lyonbot and others added 4 commits August 11, 2023 09:51

Fix broken formatting rules around namespaced JSX attributes (microso…

b35fa04

…ft#55294)

Enable eslint rules prefer-rest-params and prefer-spread (microsoft#5…

3b43d84

…5181)

Update package-lock.json

634d3a1

Adding Unit Test

dcd970e

hardikkoul requested a review from andrewbranch August 13, 2023 07:05

Update package-lock.json

fd390e7

andrewbranch reviewed Aug 14, 2023

View reviewed changes

andrewbranch changed the title ~~Prevents auto-import in non-module files~~ Prevents auto-import in module: "none" Aug 14, 2023

andrewbranch reviewed Aug 14, 2023

View reviewed changes

jakebailey and others added 10 commits August 14, 2023 12:45

Move tsserverlibrary.js to typescript.js, make tsserverlibrary.js a s…

3c6c557

…him (microsoft#55273)

Fixed hasInvalidEscape implementation (microsoft#55373)

3855369

Co-authored-by: Damien Engels <[email protected]>

Preserve input key style of computed properties in declaration emit (m…

16dab6d

…icrosoft#55298)

Add additional information section to the bug report template (micros…

f5e73d7

…oft#55360)

Organize/consolidate inlay hint tests (microsoft#55332)

05cb53e

Fix unused baseline failure on main (microsoft#55379)

5725506

Share redirects cache key calculation between multiple caches used fo…

9261ca7

…r module resolution and type reference directive (microsoft#55376)

Update package-lock.json

c5281bf

updating the Unit Test Case

3607ace

Prevent detached diagnostics from running off the end of the file (mi…

f37d2ad

…crosoft#55381)

Andarist and others added 8 commits August 26, 2023 08:57

Add a fourslash completions test related to JSDoc @template on prot…

5b9547b

…otype method (microsoft#55513)

Add a test case with a generic member call returning a closed over ob…

270a471

…ject (microsoft#55514)

Add a test for finding all references of same-named imports from two …

27a5bdd

…missing modules (microsoft#55519)

Update package-lock.json

11b2313

Add an extra test for as const satisfies with a mutable array (micr…

4b15830

…osoft#55522)

Ensure parsing context is restored in jsdoc parser (microsoft#55537)

4d7753a

Removing comment

77897bd

do not report use-before-declaration error in async like IIFE initial… (

788239f

microsoft#55279)

andrewbranch reviewed Aug 28, 2023

View reviewed changes

a-tarasyuk and others added 8 commits August 28, 2023 11:40

fix(55494): Invalid declaration with computed property using imported…

9d0dc77

… symbol (microsoft#55529)

Avoid trying to emit anonymous classish/expando functions as assignme…

5ce34ca

…nts (microsoft#55472)

Clean up helpers which are always present in ES2020 (microsoft#55515)

b5b6048

Parse reserved identifiers as dotted names of ambient modules (micros…

fe70ec9

…oft#55282)

Fixed a string completions regression when requested directly in argu…

9a6a4ab

…ment position (microsoft#55552)

Transfer all declared symbols onto the type from the interface that e…

32ebbe0

…xtends another (microsoft#55252)

Infer from parameters annotated using JSDoc (microsoft#55400)

418494e

Disallow object prototype property access on const enums (microsoft#5…

9f9a72a

…5424)

hardikkoul added 6 commits August 29, 2023 10:40

changing "compilerOptionsIndicateEs6Modules"

9ab97a6

Adding Unit Test

4dc4225

updating the Unit Test Case

23dd2d7

Removing comment

2bf66ce

adding space

8593208

Merge branch 'bug55256-Hardik' of https://github.com/hardikkoul/TypeS…

9a450cb

…cript into bug55256-Hardik

hardikkoul closed this Aug 29, 2023

hardikkoul added a commit to hardikkoul/TypeScript that referenced this pull request Aug 29, 2023

Prevents Auto import in module:none microsoft#55328

8940285

hardikkoul deleted the bug55256-Hardik branch August 29, 2023 05:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevents auto-import in `module: "none"` #55328

Prevents auto-import in `module: "none"` #55328

hardikkoul commented Aug 10, 2023

andrewbranch left a comment

hardikkoul commented Aug 11, 2023

hardikkoul commented Aug 11, 2023

andrewbranch commented Aug 11, 2023

hardikkoul commented Aug 13, 2023

hardikkoul commented Aug 13, 2023

andrewbranch Aug 14, 2023

andrewbranch Aug 14, 2023

hardikkoul Aug 20, 2023

hardikkoul Aug 27, 2023 •

edited

Loading

andrewbranch Aug 28, 2023

hardikkoul Aug 28, 2023

hardikkoul Aug 28, 2023

andrewbranch Aug 28, 2023

andrewbranch commented Aug 28, 2023

hardikkoul commented Aug 29, 2023

typescript-bot commented Aug 29, 2023

typescript-bot commented Aug 29, 2023

-verify.completions();
+verify.completions({
+  excludes: ["f"],
+  preferences: {
+    includeCompletionsForModuleExports: true
+  }
+});

	if (compilerOptions.module === ModuleKind.None){
	if (compilerOptions.module === ModuleKind.None) {

Prevents auto-import in module: "none" #55328

Prevents auto-import in module: "none" #55328

Conversation

hardikkoul commented Aug 10, 2023

andrewbranch left a comment

Choose a reason for hiding this comment

hardikkoul commented Aug 11, 2023

hardikkoul commented Aug 11, 2023

andrewbranch commented Aug 11, 2023

hardikkoul commented Aug 13, 2023

hardikkoul commented Aug 13, 2023

andrewbranch Aug 14, 2023

Choose a reason for hiding this comment

andrewbranch Aug 14, 2023

Choose a reason for hiding this comment

hardikkoul Aug 20, 2023

Choose a reason for hiding this comment

hardikkoul Aug 27, 2023 • edited Loading

Choose a reason for hiding this comment

andrewbranch Aug 28, 2023

Choose a reason for hiding this comment

hardikkoul Aug 28, 2023

Choose a reason for hiding this comment

hardikkoul Aug 28, 2023

Choose a reason for hiding this comment

andrewbranch Aug 28, 2023

Choose a reason for hiding this comment

andrewbranch commented Aug 28, 2023

hardikkoul commented Aug 29, 2023

typescript-bot commented Aug 29, 2023

typescript-bot commented Aug 29, 2023

Prevents auto-import in `module: "none"` #55328

Prevents auto-import in `module: "none"` #55328

hardikkoul Aug 27, 2023 •

edited

Loading