node-dependency-tree icon indicating copy to clipboard operation
node-dependency-tree copied to clipboard

Published 20 hours ago • verified publisher icon dependents

fix(tsConfig): path mapping by filing-cabinet

Open jjangga0214 opened this issue 4 years ago • 18 comments

The PR pending in filing-cabinet(https://github.com/dependents/node-filing-cabinet/pull/100) should be merged and published before this PR to be merged.

I personally tested this PR against https://github.com/jjangga0214/ts-yarn-lerna-boilerplate, with filing-cabinet patched by https://github.com/dependents/node-filing-cabinet/pull/100.

Closes #135

Related to:

  • https://github.com/dependents/node-filing-cabinet/pull/100
  • https://github.com/dependents/node-dependency-tree/pull/136

Thanks!

jjangga0214 avatar Aug 16 '21 04:08 jjangga0214

@mrjoelkemp Hi! Thank you for merging https://github.com/dependents/node-filing-cabinet/pull/100. Now, this transitive PR is ready to be merged as well! Thanks!

jjangga0214 avatar May 29 '22 02:05 jjangga0214

@mrjoelkemp Hi! Would you mind if I ping you? I wish this PR to be merged as I want to depend on node-dependency-tree from my new library, with this PR's feature. Thanks a lot in advance!

jjangga0214 avatar Jul 20 '22 03:07 jjangga0214

Please rebase, drop any unneeded changes and make sure lint + tests pass.

XhmikosR avatar May 05 '23 07:05 XhmikosR

how progress of this issue?

tianyingchun avatar May 26 '23 08:05 tianyingchun

@tianyingchun I will do it this week :)

jjangga0214 avatar May 26 '23 14:05 jjangga0214

@XhmikosR @tianyingchun Done it :)

jjangga0214 avatar May 27 '23 08:05 jjangga0214

Waiting new release

tianyingchun avatar May 27 '23 08:05 tianyingchun

A test is still needed.

XhmikosR avatar May 27 '23 11:05 XhmikosR

Should we add a test for cases like this as well? I'd respect your decision, but I don't think so personally, IMHO. This change is just a passing of a new single option to filing-cabinet. Even if there's a bug or inconsistency, it's not what node-dependency-tree should find out and fix. It's a dependency's job, by separation of concern and separated responsibility. The test is already created on filing-cabinet's side, and should I recreate almost the same test case for the same purpose? What do you think? Thanks :)

jjangga0214 avatar May 28 '23 00:05 jjangga0214

why we need two typescript config here?

tsConfig: config.tsConfig tsConfigPath: config.tsConfigPath

Should we make tsConfig (string/object) works correct? instead of add new option tsConfigPath? Very difficult to understand

tianyingchun avatar May 28 '23 01:05 tianyingchun

@tianyingchun I'd like to recommend reading this PR :)

jjangga0214 avatar May 28 '23 13:05 jjangga0214

@XhmikosR May I ping?

Thanks :)

jjangga0214 avatar Jun 07 '23 08:06 jjangga0214

@XhmikosR Hi? Are you super busy these days?

Thanks :)

jjangga0214 avatar Aug 25 '23 15:08 jjangga0214

@XhmikosR Hello, dear maintainers! This PR was ready-to-be-merged, but ignored for a long time and new commits came into the main branch, which is somewhat questionable. May I request more attention?

jjangga0214 avatar Nov 01 '23 12:11 jjangga0214

Hi, can the be moved forward please? 🙏

miluoshi avatar Jan 17 '24 10:01 miluoshi

I don't have time for dependents and I still think this needs a test case.

XhmikosR avatar Apr 03 '24 08:04 XhmikosR

@jjangga0214 can you write tests for the new option please?

miluoshi avatar Apr 03 '24 09:04 miluoshi