-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat: Re-enable typescript for production builds #328
Conversation
Thank you @marlonkeating for updating this and fixing the issues. Since now we have separate repo for typescript-config, i think we should move all the typescript related configurations in that repo and just update the respective changes here. |
84556bd
to
a59fb3e
Compare
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## alpha #328 +/- ##
============================
============================
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
a94c929
to
6280e10
Compare
6280e10
to
5cddfbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing on npm run lint
due to omitted .tsx
file extensions. I believe we need to modify our ESLint config to add TypeScript support so it doesn't complain about things like this.
This can be achieved with airbnb-typescript
, but would recommend adding it to our shared @edx/eslint-config
library, given that's where our ESLint config extends airbnb
and airbnb/hooks
already.
See this PR: openedx/eslint-config#124
5cddfbd
to
55fa392
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to merge into alpha
, though with a clarifying question first about the typescript
dependency and a few nits.
build: Add .ts/.tsx extensions to webpack resolver chore: Add ending newline to Image.tsx build: Use typescript config from @edx/eslint-config chore: formatting
55fa392
to
adb7300
Compare
🎉 This PR is included in version 12.9.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 12.10.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 13.1.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 15.0.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This is a fixed version of #294 (reverted in #315), with package.json that includes typescript libraries in dependencies instead of devDependencies, which ensures that the requires typescript libraries will get pulled into any consumers of openedx/frontend-build.
Testing Done
Example App
Local MFE
using typescript pulling frontend-build but no typescript libraries