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.

Sign up for GitHub

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

Jump to bottom

Progress TypeScript migration #671

Merged
merged 27 commits into from
Oct 30, 2022
Merged

Progress TypeScript migration #671

merged 27 commits into from
Oct 30, 2022

Conversation

jwhitham-ds
Copy link
Contributor

I've picked up from the starts that @Scalahansolo and @zhouzi have made. I was progressing through writing types out on another branch, but I thought that it would be good make sure that this is something that is still supported and to raise an early PR.

This PR just makes filename changes, moves devDependencies up to the root of the repo and modifies some config. Once it is approved I'll continue working on getting everything typed properly.

This branch is rebased on the latest upstream main and yarn && yarn build runs fine without errors or warnings.

image

@jwhitham-ds jwhitham-ds marked this pull request as draft August 12, 2022 05:21
@zhouzi zhouzi mentioned this pull request Aug 12, 2022
Closed
@jwhitham-ds
Copy link
Contributor Author

jwhitham-ds commented Aug 22, 2022 •
edited
Loading

Hi guys, I ran into issues trying to get Jest set up for TypeScript and then circumstances changed at work and I couldn't spend any more time on this there. I hope to spend some of my own time on this this weekend, but if anyone feels like jumping in and helping out I would welcome that.

@jwhitham-ds jwhitham-ds mentioned this pull request Aug 22, 2022
Open
@jwhitham-ds
Copy link
Contributor Author

Turns out I was most of the way there with my WIP and Jest and I just had to remove the --experimental-vm-modules option to get the tests working =D.

With that option enabled, I was getting the following error:
image

@jwhitham-ds jwhitham-ds marked this pull request as ready for review August 23, 2022 04:53
Scalahansolo and others added 17 commits August 26, 2022 12:13
@Scalahansolo @jwhitham-ds
Initial commit for converting emoji-mart to typescript
26fff40 
Co-authored-by: Josh Whitham jwhitham@digitalstack.io
@Scalahansolo @jwhitham-ds
WIP Typescript conversion
cbf7f66 
Co-authored-by: Josh Whitham jwhitham@digitalstack.io
@zhouzi @jwhitham-ds
Remove nested workspaces
0023000 
Co-authored-by: Josh Whitham jwhitham@digitalstack.io
@zhouzi @jwhitham-ds
Minimal setup
3d9f7a4 
Co-authored-by: Josh Whitham jwhitham@digitalstack.io
@jwhitham-ds
Move devDependencies to root
a974c19 
This helps with issues I ran in to surrounding @parcel/transformer-typescript-types
and is mentioned here:
missive#600
@jwhitham-ds
Remove .parcelrc
8c6318f 
The following issue was encountered, so .parcelrc was removed:
parcel-bundler/parcel#8014
@jwhitham-ds
Change import to address known issue with @parcel/transformer-typescr…
afe2ab4 
…ipt-types

parcel-bundler/parcel#5911
@jwhitham-ds
Rename files for TS and React
d5fc41b
@jwhitham-ds
Remove filetype from import
edea267
@jwhitham-ds
Change tsconfig jsx option to 'react-jsx'
38fb3f1 
https://www.typescriptlang.org/tsconfig#jsxImportSource
@jwhitham-ds
Remove strict type checking to allow repo to pass as is
e52a572
@jwhitham-ds
Remove more type checking to allow repo to pass as is
4a4a1a5
@jwhitham-ds
Remove .postcssrc
63b26d7 
As per build warning:
'@parcel/transformer-postcss: Parcel includes CSS transpilation and vendor prefixing by default. PostCSS config .postcssrc contains only redundant plugins. Deleting
it may significantly improve build performance.'
@jwhitham-ds
Run prettier
0e07e78
@jwhitham-ds
Add ts-jest package and bump jest
b7d6756
@jwhitham-ds
Add ts-jest transformers to config
ba65731
@jwhitham-ds
Remove --experimental-vm-modules option when running jest.
f2d8477 
I was getting 'ReferenceError: require is not defined' errors from jest since moving
the repo to TypeScript. Removing it allows the tests to run fine.
@TranquilMarmot
Copy link
Contributor

What's the status of this? Need help on any parts of it?

@jwhitham-ds
Copy link
Contributor Author

jwhitham-ds commented Sep 16, 2022 •
edited
Loading

This is currently waiting for review.

The PR was converted to draft because tests weren't passing, but I resolved that with my latest commit and the PR has been ready for review since then. Maybe I could have been clearer about that and also pinged the maintainers?

I was planning on waiting until this small PR was approved before I kept working on the migration.

JoshuaKGoldberg
JoshuaKGoldberg reviewed Oct 7, 2022
View reviewed changes
Copy link

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi hi! I'm not a maintainer on this repo, so there's no need to listen to my review. Just popping in out of enthusiasm for this PR landing, and to try to be helpful. 🙂

packages/emoji-mart/tsconfig.json Outdated Show resolved Hide resolved
packages/emoji-mart/tsconfig.json Outdated Show resolved Hide resolved
packages/emoji-mart/tsconfig.json Outdated Show resolved Hide resolved
jest.config.js Outdated Show resolved Hide resolved
packages/emoji-mart/src/browser.js Show resolved Hide resolved
@EtienneLem EtienneLem merged commit ff3c2aa into missive:main Oct 30, 2022
@jwhitham-ds jwhitham-ds deleted the first-pr branch October 30, 2022 21:49
@jwhitham-ds jwhitham-ds restored the first-pr branch October 30, 2022 21:49
@jwhitham-ds jwhitham-ds deleted the first-pr branch October 30, 2022 21:54
@PCPbiscuit
Copy link

Any info on when this is going to be merged?

@realtrev realtrev mentioned this pull request May 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg left review comments

@EtienneLem EtienneLem EtienneLem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jwhitham-ds @TranquilMarmot @PCPbiscuit @EtienneLem @JoshuaKGoldberg @Scalahansolo @zhouzi