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

check for .d.ts files for absolute paths #141

Conversation

davidparkagoda
Copy link
Contributor

No description provided.

@mrseanryan
Copy link
Collaborator

hi @davidparkagoda thanks for the pull request.

Can you give a description of what this PR is about?

It looks like a test to see that we don't get false positives, or errors, if importing from a d.ts file ?

@davidparkagoda
Copy link
Contributor Author

Yes that is correct. When your tsconfig does not have absolute paths (baseUrl) nor paths (aliases) we do not have this problem.

I also wanted to fix the with-paths problem however it looks like I have to send a pr request to tsconfig-paths because it does it strip .d.ts correctly.

@mrseanryan
Copy link
Collaborator

hi @davidparkagoda

I'm still not sure I understand.
Please bear with me - here is a summary of my understanding - perhaps you can read it and see if this is correct:

  1. There is a bug where there are false positives, if importing from a d.ts file
  • this only occurs if tsconfig uses absolute paths (baseUrl) or uses aliases (paths)
  • question: why does the test in this PR pass?
    does the test need to be changed, to reproduce the problem?
  1. There is another problem, involving with-paths.
    Can you describe this problem?

Thanks David,
Sean

@mrseanryan mrseanryan added help wanted need-more-info We need more information before we can process the issue bug and removed help wanted labels May 24, 2020
@davidparkagoda
Copy link
Contributor Author

Yea no problem.

  1. the second bug is with-paths. You can add the extension .d.ts in this array,

    const EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'];

    however tsconfig-paths does not strip .d from the file.

I created another pull request to fix it in tsconfig-paths to correctly strip extensions correctly regardless of how many . there are.
dividab/tsconfig-paths#124

@mrseanryan
Copy link
Collaborator

hi @davidparkagoda
thank you for the explanation.

This fix looks good.

To proceed, I would suggest:

  • amend the commit, with an entry to the CHANGELOG.md

like this:

### Changed

- .... (summary of the fix) ...

For the 2nd issue - I would suggest opening a separate issue/PR for that.

@mrseanryan
Copy link
Collaborator

Oh, and this branch needs to be rebased on master :)

@mrseanryan mrseanryan removed the need-more-info We need more information before we can process the issue label May 29, 2020
@mrseanryan
Copy link
Collaborator

mrseanryan commented Jul 5, 2020

Hi @davidparkagoda your fix is useful - so to get it released, I opened a new PR (#150), that has a CHANGELOG entry.

So I think PR can be closed.

For the bug with with-paths it could be good to have a separate PR, with suitable test.

Alternatively, If you have an example that I could use to reproduce that problem, that would be great.

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants