Skip to content

Use typescript.sys as the module resolution host #468

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

Closed
wants to merge 1 commit into from
Closed

Use typescript.sys as the module resolution host #468

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 13, 2017

But node test/comparison-tests/run-tests.js --single-test npmLink does not work.

@johnnyreilly
Copy link
Member

Ug. Any suggestions @jbrantly?

@johnnyreilly
Copy link
Member

Would using typescript.sys and falling back to the existing mechanism when it fails be a workaround that would allow us continue supporting npm link?

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Feb 16, 2017

If I remembered it correctly, referencing typescript is deliberately avoided because we don't want list TypeScript as dependency and we support other TypeScript fork implementation.

Is it still relevant today? @jbrantly @johnnyreilly what's your opinion?
Almost all forks fail to keep pace with the official compiler, leaving the official implementation the only option to use.

@jbrantly
Copy link
Member

Re: not referencing typescript, it's hard to say. I've only known 2 typescript forks, one of which is now defunct (had JSX support) and the other was originally meant to be a nightly (ntypescript) but typescript has had their own nightlies for quite a while. My leaning is that it's probably not as big of a deal.

That said, I think it's probably also not a huge deal to fix here by just doing compiler.sys (I think?)

Regarding breaking the npmLink test... I unfortunately don't have a lot of insight into that. The original PR implementing that is #141 if that's any help.

@johnnyreilly
Copy link
Member

Thanks @Andy-MS - this didn't get merged but the idea was used and shipped with v3.0.3 of ts-loader. The initial PR was a welcome and useful pointer so thank you.

@ghost ghost deleted the module_resolution_host branch October 18, 2017 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants