feat: typescript #246
feat: typescript #246
Conversation
knownasilya
force-pushed
the
typescript
branch
from
406ef68
to
a755354
Compare
knownasilya
force-pushed
the
typescript
branch
2 times, most recently
from
c76e0b0
to
762cc33
Compare
knownasilya
force-pushed
the
typescript
branch
from
762cc33
to
10ef2dd
Compare
knownasilya
force-pushed
the
typescript
branch
from
10ef2dd
to
07deadc
Compare
knownasilya
force-pushed
the
typescript
branch
2 times, most recently
from
5d2195e
to
adef064
Compare
|
I think this is ready? |
knownasilya
requested review from
bertdeblock
and removed request for
NullVoxPopuli and
chriskrycho
|
I'll take a look this week! |
|
@knownasilya can you poke at the failing CI stuff? |
- package.json: add `--bundleConfigAsCjs` to `rollup` - rollup.config.js: rename to rollup.config.mjs (as instructed from warning message) Renamed rollup.config.js to rollup.config.mjs - Updated rollup package to latest version - Added `--bundle
… call - test-app/config/ember-try: remove support for ember 3 & ember 4 lts versions - test-app/package.json: update ember-source and ember-resolver versions
- Fix ember-source version - Change `Owner` import to type import
- test-app/tests: refactor to use strictEqual instead of equal - test-app/package.json: remove unused dependencies
- docs/package.json: update ember-source version, add @ember/strings
- addon/src/services/page-title: add conditional import for getOwner depending on ember version - test-app/config/ember-try: add ember 4.8 lts configuration to test list
knownasilya
requested review from
chriskrycho and
NullVoxPopuli
and removed request for
bertdeblock and
NullVoxPopuli
|
@NullVoxPopuli @chriskrycho just waiting on a review to get this across the finish line. |
| @@ -34,13 +34,13 @@ jobs: | |||
| matrix: | |||
| os: [ubuntu-latest] | |||
| browser: [chrome, firefox] | |||
| node: ['14', '16', '18'] | |||
There was a problem hiding this comment.
making note of braking change: dropping node 14
nevermind, this isn't a breaking change.
ember-page-title is a v2 addon so testing against node doesn't matter.
We could have gotten rid of the whole node matrix here
| ember-release, | ||
| ember-beta, | ||
| ember-classic, |
There was a problem hiding this comment.
dropping support for ember-classic
| ember-release, | ||
| ember-beta, | ||
| ember-classic, | ||
| embroider-safe, |
There was a problem hiding this comment.
why was embroider removed? can we add them back?
| }, | ||
| ], | ||
| }); | ||
| return app.toTree(); |
There was a problem hiding this comment.
why'd we get rid of embroider here?
| @@ -15,6 +15,7 @@ | |||
| !.* | |||
| .*/ | |||
| .eslintcache | |||
| /dev-only-types/ | |||
There was a problem hiding this comment.
v2-addon convention is unpublished-development-types, fwiw
| @@ -242,37 +316,52 @@ export default class PageTitleService extends Service { | |||
| * Find token by id | |||
| * | |||
| * IE11 compatible approach due to lack of Array.find support | |||
| * TODO: Remove when we drop support for Ember 3.28 | |||
There was a problem hiding this comment.
can you open an issue about this, so we don't forget?
| // But fallback for type-overrides and such | ||
| "*": ["dev-only-types/*"] | ||
| }, | ||
| "skipLibCheck": true |
| @@ -2,12 +2,15 @@ | |||
|
|
|||
| module.exports = { | |||
| root: true, | |||
| parser: 'babel-eslint', | |||
| parser: '@babel/eslint-parser', | |||
There was a problem hiding this comment.
test app isn't using TS?
| }, | ||
| }, | ||
| }, | ||
| embroiderSafe(), |
There was a problem hiding this comment.
let's bring these back
| }); | ||
|
|
||
| const { maybeEmbroider } = require('@embroider/test-setup'); | ||
| return maybeEmbroider(app, { |
There was a problem hiding this comment.
let's bring back embroider
|
@leepfrog I don't have time to address this feedback today. If you have time feel free to do that, otherwise I'll tackle it later this week. |
|
@bertdeblock if you have time to address feedback feel free to pr against this branch |
Closes #239