Skip to content

Conversation

leaexplores
Copy link

Hi guys,

I was reading ts-loader source code recently and I stumbled upon some magic compile numbers.

It feels like we want to use the official typescript constants instead of some magic number.

This will allow to future proof us is case they change and feels easier to read.

Thanks for looking into it !

@jbrantly
Copy link
Member

Thanks for contributing! The magic numbers are actually done this way on purpose so that TypeScript is not required in the generated JS file. For example...

// index.ts
import typescript = require('typescript')

var a = 1;

// index.js
var a = 1;

But if you reference an enum:

// index.ts
import typescript = require('typescript')

var a = typescript.ModuleKind.CommonJS;

// index.js
var typescript = require('typescript'); // this line is emitted but we don't want it

var a = 1;

Looks like this is being tracked in microsoft/TypeScript#4690

@leaexplores
Copy link
Author

Oh I see. Seems like we're going to have to wait a few typescript release before merging this :)

Do you want me to leave it open so that when microsoft/TypeScript#4690 resolves we'll be able to merge it ? Or open a tracking issue ?

Thanks!

@jbrantly
Copy link
Member

We can just leave it open.

@leaexplores
Copy link
Author

Cool.

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Nov 7, 2016

@ddrmanxbxfr Thanks for your contribution! But ts-loader has a total revamp recently. Would you mind re-submitting a pull request for this?

@johnnyreilly
Copy link
Member

Following the change to the enum in 2.0 and @HerringtonDarkholme painful experience we've decided not to do this.

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.

4 participants