-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor(model-form): Glimmer Components #539
base: staging
Are you sure you want to change the base?
Conversation
@guidojw what do you mean by yarn install being broken? is that why the build fails? |
It installs two versions of the same webpack/webpack#15772 should fix this. (also this PR is far from finished) |
I did run into some build errors yeah, I just tried testing this PR and it tells me exactly what you mentioned, duplicate declarations of the same identifiers. I'll wait with reviewing this until you think it's ready then. |
Yeah I will undraft it when it's ready 👍🏿 |
…efactor/glimmer-components-model-form � Conflicts: � package.json � yarn.lock
This now builds |
…l-form' into refactor/glimmer-components-model-form
f4826b3
to
c143779
Compare
If you don't mind, I will separate this from the refactor to glimmer. I like the idea of using typescript for our components, but for the sake of getting up to date with other dependencies we need to upgrade our ember version asap. In contrast, TS is more of a nice to have. Adding the introduction of TS as a todo in #429 |
ref #530
Summary
Realised in #529 that the model-form components need to be refactored quite a lot so thought it would be a good idea to do this in a separate PR. This is also the PR that introduces TypeScript via
ember-cli-typescript
, because I was gonna do the new components in TypeScript.Other information
@types/eslint-scope
's resolution is overridden to^7.0.0
becausewebpack
has a bug where@types/eslint-scope
is in its dependencies instead of devDependencies, this can be removed from the package.json when webpack/webpack#15772 is resolved.