jellyjs / angular2-file-drop

Angular2 component with Drag and Drop support for files
23 stars 18 forks source link

Replace @types/es6-shim with es6-shim #12

Closed SamoB25 closed 7 years ago

SamoB25 commented 7 years ago

Replaces @types/es6-shim with es6-shim library. The main reason is that it is better maintained. I'd like to ask @barbatus to check this change as well, as he was the one to introduce AOT to angular2-file-drop.

If it's OK, I'd like to urge @kamilkisiela to release this patch as soon as possible, as it is quite needed. Thanks!

kamilkisiela commented 7 years ago

@types/es6-shim and es6-shim are two different packages. First one has type declarations but second one has polyfills.

itsnotvalid commented 7 years ago

Just curious, I fixed problems with AOT without using rollup in my fork, which then you can get away without even es6-shim. The key is you can add a line in tsconfig.json:

{
  "compilerOptions": {
    "lib": ["dom", "es2015"]
  }
}

(or es6 in place of es2015)

kamilkisiela commented 7 years ago

Little explanation.

First thing. compilerOptions.lib is an option that allows you to provide built-in types. So enabling es2015 is pretty much the same as using @types/es6-shim. I mean, they have the same purpose which is giving the declarations but these are totally different packages.

Second thing. Rollup have nothing to do with AoT because it's just a bundler. In this project, TypeScript generates bunch of files that are es6 modules and then rollup merge them together into a single file and converts them into AMD and CommonJS. This bundle lives under main field in package.json, rest of the modules provided by TypeScript are defined in module field. It means that if you use Rollup or webpack (v2) it takes es6 modules defined in module field to generate a bundle and also to do tree-shaking :)

SamoB25 commented 7 years ago

You are absolutely right @kamilkisiela. I had a problem because I have es6-shim imported into the project and @types/es6-shim and es6-shim clash when it comes to typings (using webpack):

ERROR in /usr/src/app/typings/globals/es6-shim/index.d.ts
(3,14): error TS2300: Duplicate identifier 'PropertyKey'.

ERROR in /usr/src/app/node_modules/@types/es6-shim/index.d.ts
(6,14): error TS2300: Duplicate identifier 'PropertyKey'.

Would it be perhaps OK to remove @types/es6-shim from angular2-file-drop?

kamilkisiela commented 7 years ago

@SamoB25 I think we can use es2015 in compilerOptions.lib and remove @types/es6-shim. Could you test it and prepare a PR?

SamoB25 commented 7 years ago

Will do :)

SamoB25 commented 7 years ago

Closing this one.