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

warning 6133 when namespace declaration follows import #33317

Closed
JirkaDellOro opened this issue Sep 9, 2019 · 20 comments
Closed

warning 6133 when namespace declaration follows import #33317

JirkaDellOro opened this issue Sep 9, 2019 · 20 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@JirkaDellOro
Copy link

JirkaDellOro commented Sep 9, 2019

tsconfig.json: "noUnusedLocals": true,
tsc-version: Version 3.6.2

import * as Http from "http";
import * as Url from "url";

namespace Test {
 ...
}

Result: Warning 6133 'Test' is declared but its value is never read.
Expected: No Warning

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 9, 2019

You're not exporting the namespace.
Are you using it inside the file?

If you aren't, then it makes sense you're getting the error message

@JirkaDellOro
Copy link
Author

I'm using the namespace solely for separation of, well, namespaces. It's not about building a module. So I don't think exporting it would make sense.

@AnyhowStep
Copy link
Contributor

Are you referencing Test.someExportedName inside the file?

@JirkaDellOro
Copy link
Author

No, it shouldn't be necessary to address anything inside the namespace explicitely with the full identifier including the namespace. I don't have exported members inside the namespace also, and it wouldn't change the behaviour. At this point of time I can either add

  • //@ts-ignore no-unused-variable
  • export to the namespace

to suppress the behaviour, both of which are not very elegant.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 9, 2019

The namespace is not necessary, then.

Your file itself acts as its own namespace...

unexported variables inside the file should not leak into other files.
I don't see why you're creating a namespace when you don't need one.


Given the following,

namespace Test {
    export const myVar = 32;
}

Playground

The following JS is emitted,

"use strict";
var Test;
(function (Test) {
    Test.myVar = 32;
})(Test || (Test = {}));

As you can see, during run-time, a Test variable is created.
A namespace is not a strictly compile-time construct.

It creates an identifier in both the type space and value space.

It is a fact that you're not using the variable Test after declaring it.


You shouldn't use namespaces anymore. They're sort of deprecated.
Kept around for backwards compatibility.

@fatcerberus
Copy link

Like @AnyhowStep said, if you're using ES modules (which you are since your example has import) then you don't need the namespace - your declarations won't leak outside the file you declared them in. It's different from the olden days where everything was global by default.

@fatcerberus
Copy link

fatcerberus commented Sep 9, 2019

@JirkaDellOro It’s fairly likely the error is alerting you to a larger issue: because you’ve added import statements, TS now sees the file as an ES module and therefore nothing in Test is accessible from any other file unless you export it. It’s therefore dead code and the compiler is warning you about it.

@JirkaDellOro
Copy link
Author

@AnyhowStep

You shouldn't use namespaces anymore. They're sort of deprecated. Kept around for backwards compatibility.

Can this statement be found in the official documentation or is it an official guideline?


The namespace in this case is necessary not for separating things at runtime, but at designtime. I think @fatcerberus second answer comes closer to addressing the problem

TS now sees the file as an ES module and therefore nothing in Test is accessible from any other file unless you export it.

As a developer, I should be telling TS if the file is supposed to become a module or not, and don't expect TS to see the file as such because imports are used. So this behaviour appears faulty.

@fatcerberus
Copy link

import being present means the file is an ES module, by definition. You can’t use that syntax in normal scripts, and there’s no way to transpile it to one in a way that wouldn’t violate the ES specification.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 9, 2019

#30994 (comment)

I'm just going to selectively quote but you should read the whole discussion.

Namespaces are probably never going away and, simultaneously, you probably shouldn't use them, since better, more standard and modern patterns exist through the use of modules.

but most of the time you don't need one, as we now have a different conceptual organizational model recommended for large-scale JS (modules).

most new TS code should probably never need to use them, and should think hard about if they really need to.


#30994 (comment)

If you're considering using namespaces for code organization: Don't. Modules have subsumed that role.

@fatcerberus
Copy link

fatcerberus commented Sep 9, 2019

To be clear: ESM, at runtime, is a separate execution model compared to execution of regular scripts, and import is only supported when the engine (or transpiler) is running in the former mode. You thus can't get away from the local-scoping rules when using import, TS would just introduce runtime errors if it were to ignore them.

@JirkaDellOro
Copy link
Author

JirkaDellOro commented Sep 9, 2019

@AnyhowStep: Thanks for pointing me there. In the discussion, the person you cite also stated this:

one of the major features of namespaces in the traditional sense is cross-file scope merging,

That is one of the reasons I use it, it's not about building a module. I'd use module then.


@fatcerberus:

import being present means the file is an ES module, by definition. You can’t use that syntax in normal scripts, and there’s no way to transpile it to one in a way that wouldn’t violate the ES specification.

Would you mind to point me to that specification? I can't find it stated like this in the official specs. Also the compiled code (using ts-ignore) actually works fine and as expected, it's just that the compiler warning doesn't seem to fit or is needless.

PS: it's a warning, not an error that would stop compilation. I changed the title of this issue to reflect that.

@JirkaDellOro JirkaDellOro changed the title error 6133 when namespace declaration follows import warning 6133 when namespace declaration follows import Sep 9, 2019
@JirkaDellOro
Copy link
Author

Could someone from the TypeScript-Team please comment on this?

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 12, 2019
@RyanCavanaugh
Copy link
Member

It's the intended behavior

@JirkaDellOro
Copy link
Author

@RyanCavanaugh: Thanks for tuning in.

So I understand the intended behaviour must be to print a warning that the namespace is not read, when its contents accesses imported modules. I can't grasp the idea behind that intension, though. Why does using imports change the meaning or behaviour of the namespace? There is no warning without imports and the compiled code works as intended also with the imports.

I'd love to have an explanation I can tell my students so that they know why they have to use export or disable the compilerwarning.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@JirkaDellOro
Copy link
Author

@RyanCavanaugh ?

@JirkaDellOro
Copy link
Author

To be clear: ESM, at runtime, is a separate execution model compared to execution of regular scripts, and import is only supported when the engine (or transpiler) is running in the former mode. You thus can't get away from the local-scoping rules when using import, TS would just introduce runtime errors if it were to ignore them.

@fatcerberus : Do you know where I can look that up?

@fatcerberus
Copy link

fatcerberus commented Sep 18, 2019

The pain due to ESM being a separate execution model from normal scripts is why we ended up with .mjs in Node and type=module in browsers. It's all very well documented at this point with just a quick Google search.

But here's what the ECMAScript spec has to say anyway:

Notice that only the Module grammar allows ImportDeclaration and ExportDeclaration productions. Therefore when your script contains import TS automatically switches to Module mode because that's the only valid interpretation: it'd be a syntax error at runtime if you tried to run that as a script. And modules have their own top-level scope.

See also:

"Global code" doesn't allow import and export--though that's not stated explicitly there, it's implied by the grammar. "Module code" does.

If that Test namespace is somehow accessible from other files despite the import statements, then either:

  1. Your runtime environment is not spec-compliant, OR
  2. The imports happen to have been elided by TS because they aren't used in the code

The "module" option in tsconfig, to be clear, only affects what TS generates as output. It doesn't affect the semantics of the code you write. TS follows the ES specification as closely as possible in the untranspiled code, and ES says import = module.

@JirkaDellOro
Copy link
Author

@fatcerberus Thank you very much for your efforts, that gives me some hints to come up with an explanation for my students. I had to decide which workaround to recommend

At this point of time I can either add

  • //@ts-ignore no-unused-variable
  • export to the namespace

to suppress the behaviour, both of which are not very elegant.

I go with export now and stick to "by definition", though it still should work and works to use a module in global code without exporting. At least, export does no harm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants