Convert to v2 addon#296
Convert to v2 addon#296chriskrycho merged 3 commits intoember-modifier:masterfrom SergeAstapov:v2-addon
Conversation
|
@chriskrycho if you'd like, I can split PR into two:
|
|
closes #102 |
|
Will review (at last!!!) tomorrow! |
chriskrycho
left a comment
There was a problem hiding this comment.
Thanks so much! Bunch of small tweaks, and we should be able to land this!
| "exports": { | ||
| ".": "./dist/index.js", | ||
| "./*": "./dist/*", | ||
| "./addon-main.js": "./addon-main.js" | ||
| }, | ||
| "typesVersions": { | ||
| "*": { | ||
| ".": [ | ||
| "./dist/index.d.ts" | ||
| ], | ||
| "*": [ | ||
| "./dist/*" | ||
| ], | ||
| "./-private/signature": [ | ||
| "./dist/-private/signature.d.d.ts" | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
Overall I'm fine with landing this as is for now, but we may want to revisit. Among other things, it'd be good not to expose the -private bit at all.
There was a problem hiding this comment.
@chriskrycho the only reason for exposing -private/signature is used in tests to import DefaultSignature, NamedArgs, PositionalArgs and seem like DefaultSignature is not available for import from ember-modifier.
Do you think we can make DefaultSignature importable?
There was a problem hiding this comment.
We should not make it importable, and we should use import maps to make the others available for import instead!
There was a problem hiding this comment.
Again, though, it's fine to land this way for now; we can tweak it after landing the PR!
There was a problem hiding this comment.
@chriskrycho agree! there is always room for improvement :)
at your convenience, please give another look. I think I've addressed everything you called out. |
chriskrycho
left a comment
There was a problem hiding this comment.
👏🏼 Thank you so much for all of this work! Let's do it!
|
@chriskrycho btw this is breaking change, as it requires consuming app/addon to have ember-auto-import v2 or consuming side should be v2 addon hence may worth add respective label |
|
Doing so! Thanks! |
Last part of #203
Follows Porting an Addon to V2 guide