You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[spec] Grammar does not allow ambients inside an internal module #533
This is just an oversight in the grammar, but the ModuleElement production does not go to AmbientDeclaration on the right side, which implies that an internal module cannot contain ambients. It needs to be added to the RHS.
Actually you need to add AmbientDeclaration, but not including AmbientExternalModuleDeclaration.
But ambient declarations inside nonambient modules is allowed by both compilers, and we have been supporting it. According to Ryan Cavanaugh (@RyanCavanaugh) and Dan Quirk (@danquirk) there are real world customers who depend on this.
It's also very useful when you want to directly use node.js's require without having to pollute the global scope with it (this is a very real problem when using node.d.ts with any of the compiler .d.ts files at the same time, for example).
Also, something else. The grammar was changed to remove AmbientExternalModuleDeclaration from AmbientDeclaration. However, the forbidding of nested ambient external modules appears to be in the semantic phase. So technically, the grammar should allow it by including AmbientExternalModuleDeclaration in AmbientDeclaration.
I'd prefer to keep AmbientExternalModuleDeclaration out of AmbientDeclaration so that the grammar reflects the restriction. The fact that the compiler verifies it in the semantic phase is really just an implementation detail.
Activity
ahejlsberg commentedon Aug 31, 2014
This is by design. Ambient declarations are only allowed at the top level (see sections 11.1 and 12.1 in the spec).
JsonFreeman commentedon Sep 2, 2014
But ambient declarations inside nonambient modules is allowed by both compilers, and we have been supporting it. According to Ryan Cavanaugh (@RyanCavanaugh) and Dan Quirk (@danquirk) there are real world customers who depend on this.
RyanCavanaugh commentedon Sep 2, 2014
e.g.
https://github.com/BabylonJS/Babylon.js/blob/master/Babylon/Physics/babylon.physicsEngine.ts
https://github.com/BabylonJS/Babylon.js/blob/master/Babylon/Physics/Plugins/babylon.cannonJSPlugin.ts
It's also very useful when you want to directly use node.js's
requirewithout having to pollute the global scope with it (this is a very real problem when using node.d.ts with any of the compiler .d.ts files at the same time, for example).ahejlsberg commentedon Sep 2, 2014
Ok, got it. Will fix the spec.
JsonFreeman commentedon Sep 6, 2014
This was updated in section 10.2, but still needs to be updated in the Grammar appendix.
JsonFreeman commentedon Sep 6, 2014
Also, something else. The grammar was changed to remove AmbientExternalModuleDeclaration from AmbientDeclaration. However, the forbidding of nested ambient external modules appears to be in the semantic phase. So technically, the grammar should allow it by including AmbientExternalModuleDeclaration in AmbientDeclaration.
ahejlsberg commentedon Sep 6, 2014
I'd prefer to keep AmbientExternalModuleDeclaration out of AmbientDeclaration so that the grammar reflects the restriction. The fact that the compiler verifies it in the semantic phase is really just an implementation detail.