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

BUG? Typescript export namespace nested Multiple exports of name #1300

Closed
NewFuture opened this issue Mar 13, 2019 · 8 comments · Fixed by #1320
Closed

BUG? Typescript export namespace nested Multiple exports of name #1300

NewFuture opened this issue Mar 13, 2019 · 8 comments · Fixed by #1320

Comments

@NewFuture
Copy link

import/export

export namespace API {
    //Multiple exports of name 'User' 👇
    export namespace User {
        export const A = 0;
    }

    export namespace M {
        //Multiple exports of name 'User' 👇
        export const User = {};
    }
}

But they are not the same User

@ljharb
Copy link
Member

ljharb commented Mar 13, 2019

I'm not familiar with how namespaces work (since that's not a concept that exists in javascript) but clearly it seems like one is a sibling of M, and one is nested under M.

Do you think the rule should understand namespaces, or do you think it should ignore them?

@NewFuture
Copy link
Author

it will be translated to

exports.API = {
  User:{
    A:0
  },
  M:{
    User:{}
  }
}
  • the first User's full name is API.User
  • the second User's full name is API.M.User

If namespaces can be parsed as above it will be resolved.

@ljharb
Copy link
Member

ljharb commented Mar 13, 2019

In that case, there's only one export - API - and all the others are just object properties.

So, it seems like this plugin should ignore everything but the top-level one.

@NewFuture
Copy link
Author

yes, it's ok.

@conorcussell
Copy link

@ljharb I'm experiencing this and happy to try and submit a PR to fix it. I'll start poking around the codebase anyway, but if you had any suggestions of what would need to change to enable this then that would help. Thanks!

@conorcussell
Copy link

I guess we could add a check to addNamed in the export rule.

Would something like this be a terrible idea?

function addNamed(name, node) {
  if (node.parent.type !== 'Program') return

  ... // rest of the code
}

@ljharb
Copy link
Member

ljharb commented Mar 20, 2019

I'm not very knowledgeable on this part of the code; but it doesn't seem like skipping any top-level named export would be ideal :-)

@conorcussell
Copy link

@ljharb that example would do the opposite, but I'm sure it'll break something else instead. Was just hoping for some pointers of how this sort of thing would usually be approached. I'll see if I can take a proper look sometime in the next week 😀

ljharb added a commit that referenced this issue Apr 12, 2019
[fix] `export`: Support typescript namespaces

Fixes #1300.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants