← 输入框|github.com|刷新
Skip to content

[spec] Grammar does not allow ambients inside an internal module #533

Closed

Description

Courtesy of Jason Freeman (@JsonFreeman)

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.

Activity

  1. added this to the milestone on Aug 26, 2014
  2. modified the milestones: , TypeScript 1.1 on Aug 29, 2014
  3. ahejlsberg commented on Aug 31, 2014

    @ahejlsberg
    Member

    This is by design. Ambient declarations are only allowed at the top level (see sections 11.1 and 12.1 in the spec).

  4. JsonFreeman commented on Sep 2, 2014

    @JsonFreeman
    Contributor

    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.

  5. RyanCavanaugh commented on Sep 2, 2014

    @RyanCavanaugh
    Member

    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 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).

  6. ahejlsberg commented on Sep 2, 2014

    @ahejlsberg
    Member

    Ok, got it. Will fix the spec.

  7. JsonFreeman commented on Sep 6, 2014

    @JsonFreeman
    Contributor

    This was updated in section 10.2, but still needs to be updated in the Grammar appendix.

  8. JsonFreeman commented on Sep 6, 2014

    @JsonFreeman
    Contributor

    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.

  9. ahejlsberg commented on Sep 6, 2014

    @ahejlsberg
    Member

    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.

  10. locked and limited conversation to collaborators on Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugA bug in TypeScriptFixedA PR has been merged for this issueSpecIssues related to the TypeScript language specification

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions