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

Typescript fix: export function Region #421

Merged
merged 1 commit into from
May 30, 2018
Merged

Typescript fix: export function Region #421

merged 1 commit into from
May 30, 2018

Conversation

adrw
Copy link
Contributor

@adrw adrw commented May 25, 2018

I started working on a FrintJS app using the multiple-apps as starter code. I began by moving the repo to Typescript and encountered this persistent error in Root.jsx when Region is imported.

Module '"/Users/aparadi/Development/misk-admin/main/node_modules/frint-react/index"' has no exported member 'Region'.

To fix this, Region is added as an export to the index definition file and the error no longer appears.

I'm still early in my Typescript days so if this isn't the best way to resolve the error, please comment below.

@fahad19
Copy link
Member

fahad19 commented May 25, 2018

thanks for your valuable contribution, @andrewparadi!

could you please also try something like this as mentioned in this comment? the more strict the better :) #410 (comment)

we also have a placeholder issue #412 for migrating all our packages to TypeScript. if you feel like taking a stab at them, please feel free :)

@codecov
Copy link

codecov bot commented May 25, 2018

Codecov Report

Merging #421 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #421   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files         111      111           
  Lines        4251     4251           
=======================================
  Hits         4155     4155           
  Misses         96       96

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 238c803...3879ba5. Read the comment docs.

@adrw
Copy link
Contributor Author

adrw commented May 25, 2018

Thanks for the heads up to those issues @fahad19, I'll plan to make other PRs under #412 as I come across any other Typescript issues in the weeks to come as I work on our new FrintJS app.

A huge thank you already for FrintJS, exactly what we've been searching for, very thankful I stumbled upon it instead of having to try and build it all from scratch.

Once it's ready to merge, let me know and I can squash the commits with rebase.

@adrw
Copy link
Contributor Author

adrw commented May 25, 2018

Commit squashed and ready for merge if no other changes are requested.

@fahad19 fahad19 requested a review from a team May 26, 2018 10:41
@fahad19
Copy link
Member

fahad19 commented May 26, 2018

@frintjs/core: please take a look and approve :D

@fahad19
Copy link
Member

fahad19 commented May 26, 2018

@leandrooriente: please let me know if you would be willing to merge and release a new patch version.

@fahad19 fahad19 merged commit 8c976f8 into frintjs:master May 30, 2018
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.

4 participants