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

refactor(model-form): Glimmer Components #539

Draft
wants to merge 22 commits into
base: staging
Choose a base branch
from

Conversation

guidojw
Copy link
Member

@guidojw guidojw commented May 10, 2022

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 because webpack 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.

@DrumsnChocolate
Copy link
Contributor

@guidojw what do you mean by yarn install being broken? is that why the build fails?

@guidojw
Copy link
Member Author

guidojw commented May 18, 2022

@guidojw what do you mean by yarn install being broken? is that why the build fails?

It installs two versions of the same @types package (@types/eslint-scope to be specific) and thus TypeScript building fails because of overlapping types. I fixed yarn.lock to have it not do that which works as a temporary fix in dev.
However because of the --immutable flag and yarn wanting to update yarn.lock because it's technically incorrect, the command in CI here fails.

webpack/webpack#15772 should fix this.

(also this PR is far from finished)

@DrumsnChocolate
Copy link
Contributor

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.

@guidojw
Copy link
Member Author

guidojw commented May 18, 2022

Yeah I will undraft it when it's ready 👍🏿

@guidojw
Copy link
Member Author

guidojw commented Dec 31, 2022

This now builds

@guidojw guidojw force-pushed the refactor/glimmer-components-model-form branch 4 times, most recently from f4826b3 to c143779 Compare December 31, 2022 21:15
@DrumsnChocolate
Copy link
Contributor

DrumsnChocolate commented Nov 9, 2024

I think I will breathe some new life into this PR once #635 and #896 are merged

@DrumsnChocolate
Copy link
Contributor

DrumsnChocolate commented Nov 9, 2024

This is also the PR that introduces TypeScript via ember-cli-typescript, because I was gonna do the new components in TypeScript.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants