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

Package is not formatted according to Angular Package Format 4.0 Spec #9

Open
steveblue opened this issue Sep 25, 2017 · 7 comments
Open

Comments

@steveblue
Copy link

steveblue commented Sep 25, 2017

This package is not formatted according to Package Format 4.0. Please update this package to follow the spec.

https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/preview

@vknabel
Copy link
Contributor

vknabel commented Sep 26, 2017

@steveblue Do you currently have problems using conclurer/markdown-to-html-pipe?

@vknabel vknabel self-assigned this Sep 26, 2017
@vknabel
Copy link
Contributor

vknabel commented Nov 4, 2017

@steveblue we will migrate to the package format once problems occur.

@vknabel vknabel closed this as completed Nov 4, 2017
@steveblue
Copy link
Author

steveblue commented Nov 4, 2017

@vknabel The package spec is there to ensure a contract between library developers and Angular. I couldn't import into my app as-is because it doesnt conform to the Package Spec, which would make your package compatible with Closure Compiler. In the future when ABC is released your library will not work with ABC. It already doesnt work with Closure Compiler. Its pretty easy to make it compatible, not sure why you are resisting this change.

@steveblue
Copy link
Author

steveblue commented Nov 4, 2017

This is primary because your tsconfig is incorrectly formatted. You should be using something like this:

ES2015

{
  "compilerOptions": {
    "target": "es2015",
    "module": "es2015",
    "declaration": true,
    "stripInternal": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "noImplicitAny": false,
    "removeComments": true,
    "allowUnreachableCode": false,
    "moduleResolution": "node",
    "outDir": "./ngfactory",
    "lib": ["es2015", "dom"],
    "skipLibCheck": true,
    "types": []
  },
  "angularCompilerOptions": {
   "genDir": "./ngfactory",
   "strictMetadataEmit": true,
   "skipTemplateCodegen": true,
   "annotateForClosureCompiler": true,
   "flatModuleOutFile": "default-lib.js",
   "flatModuleId": "default-lib",
   "debug": false
  },
  "files": [
    "./tmp/index.ts"
  ],
  "compileOnSave": false,
  "buildOnSave": false
}

ES5

{
  "compilerOptions": {
    "target": "es2015",
    "module": "es5",
    "declaration": true,
    "stripInternal": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "noImplicitAny": false,
    "removeComments": true,
    "allowUnreachableCode": false,
    "moduleResolution": "node",
    "outDir": "./ngfactory",
    "lib": ["es2015", "dom"],
    "skipLibCheck": true,
    "types": []
  },
  "angularCompilerOptions": {
   "genDir": "./ngfactory",
   "strictMetadataEmit": true,
   "skipTemplateCodegen": true,
   "annotateForClosureCompiler": true,
   "flatModuleOutFile": "default-lib.js",
   "flatModuleId": "default-lib",
   "debug": false
  },
  "files": [
    "./tmp/index.ts"
  ],
  "compileOnSave": false,
  "buildOnSave": false
}

@vknabel vknabel reopened this Nov 4, 2017
@vknabel
Copy link
Contributor

vknabel commented Nov 4, 2017

@steveblue I am sorry for closing this issue, but I didn‘t know of the closure compiler compatibility problems or any benefits adopting the package format.
Now as I know of it, I will have a deeper look on this.

@steveblue
Copy link
Author

steveblue commented Nov 5, 2017

Using the package spec is only one step to make this compatible with CC, but it is a huge step.

Other things that need to happen:

import * as marked from 'marked'; cannot be interpreted by CC, at least not yet, so marked needs to be an 'extern' (external script) in the CC world. The problem here is *. If it were import { marked } from 'marked' this would be OK, and marked doesn't have to be an extern. But AFAIK this is not possible with marked because it is distributed as commonjs module. module.exports = require('./lib/marked');

This would mean users of the pipe need to package marked on their own, but that is a good thing because now the pipe is bundler agnostic.

To make TS happy, you could do the following instead:

declare let marked: MarkedStatic;

CC has crazy optimizations that it does to a codebase. It mangles to the nth degree, but CC doesn't mangle strings. Since marked has to be an external script, CC cannot mangle any calls to methods on marked, but it tries to anyways, unless the method is called using bracket syntax.

    public static setOptions(options: MarkedOptions): void {
        marked['setOptions'](options);
    }

I could submit this as a PR, but the change required that removes the marked import is a big change.

@macjohnny
Copy link
Contributor

the easiest way to comply with the Angular Package specification (and hence avoid any possible problems arising from differing to it) is to use https://github.com/dherges/ng-packagr
it is as easy as creating a public_api.ts file and configuring the library build.

@vknabel vknabel removed their assignment Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants