Skip to content

Incrementally introduce TypeScript language#101

Merged
sarayourfriend merged 4 commits intomasterfrom
try/incremental-ts-lang
Nov 13, 2020
Merged

Incrementally introduce TypeScript language#101
sarayourfriend merged 4 commits intomasterfrom
try/incremental-ts-lang

Conversation

@sarayourfriend
Copy link
Copy Markdown
Collaborator

@sarayourfriend sarayourfriend commented Nov 12, 2020 •
edited
Loading

Testing instructions

Checkout the branch and run tsc --build and ensure there are no errors. Manually check utils/types and ensure that they're being built correctly and as expected (especially anything re-exporting lodash).

For some reason this breaks the pre-commit hook in this manner:

📦  Zero test...

Configuration error:

Could not locate module @wp-g2/substate/types mapped as:
/Users/sara/projects/g2/packages/substate/types/src.

Please check your configuration for these entries:
{
  "moduleNameMapper": {
    "/^@wp-g2\/(.*)$/": "/Users/sara/projects/g2/packages/$1/src"
  },
  "resolver": null
}
    at createNoMappedModuleFoundError (/Users/sara/projects/g2/node_modules/jest-resolve/build/index.js:501:17)
    at Resolver.resolveStubModuleName (/Users/sara/projects/g2/node_modules/jest-resolve/build/index.js:462:21)
    at Resolver.getMockModule (/Users/sara/projects/g2/node_modules/jest-resolve/build/index.js:304:31)
    at /Users/sara/projects/g2/node_modules/jest-resolve-dependencies/build/index.js:67:45
    at Array.reduce (<anonymous>)
    at DependencyResolver.resolve (/Users/sara/projects/g2/node_modules/jest-resolve-dependencies/build/index.js:53:25)
    at DependencyResolver.resolveInverseModuleMap (/Users/sara/projects/g2/node_modules/jest-resolve-dependencies/build/index.js:175:30)
    at DependencyResolver.resolveInverse (/Users/sara/projects/g2/node_modules/jest-resolve-dependencies/build/index.js:198:17)
    at SearchSource.findRelatedTests (/Users/sara/projects/g2/node_modules/@jest/core/build/SearchSource.js:280:30)
    at SearchSource.findRelatedTestsFromPattern (/Users/sara/projects/g2/node_modules/@jest/core/build/SearchSource.js:345:19)
husky > pre-commit hook failed (add --no-verify to bypass)

@vercel
Copy link
Copy Markdown

vercel bot commented Nov 12, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/itsjonq/g2/3s739vx86
✅ Preview: https://g2-git-try-incremental-ts-lang.itsjonq.vercel.app

@sarayourfriend sarayourfriend mentioned this pull request Nov 12, 2020
Closed
@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 12, 2020

For some reason this breaks the pre-commit hook in this manner:

Oh no... built tool stuff 🙈

Is it because the types/index.d.ts file is deleted?

That aside.. I'm not sure how to review this PR 😊

@sarayourfriend
Copy link
Copy Markdown
Collaborator Author

sarayourfriend commented Nov 12, 2020 •
edited
Loading

Is it because the types/index.d.ts file is deleted?

It's deleted from git because now it's being auto-generated. Locally the file does exist due to the build 🤔 Why does jest care about the test files though?

@ItsJonQ I added review instructions. @sirreal should be able to to help as well whenever he has some time 🙂

@sarayourfriend sarayourfriend self-assigned this Nov 12, 2020
v9ai
v9ai reviewed Nov 13, 2020
View reviewed changes
tsconfig.base.json
@@ -0,0 +1,34 @@
{
"compilerOptions": {
"allowJs": true,
Copy link
Copy Markdown

@v9ai v9ai Nov 13, 2020

Choose a reason for hiding this comment

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

❤️

@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 13, 2020

@saramarcondes Oh wow! Look at them types 😍

I'm not too sure the pre-commit hook / jest would be made at your changes 🙈 .

Maybe it's okay from this point forward? (I'm guessing you committed the stuff with --no-verify)

@sirreal sirreal force-pushed the try/incremental-ts-lang branch from fb0f6c5 to f440976 Compare November 13, 2020 15:05
sirreal
sirreal approved these changes Nov 13, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

I rebased to fix a conflict. Also:

  • Improve types directory ignores
  • Exclude non-generated types directories from ignore
  • Add type clean to clean script

Let's land this and see how it goes, thanks everyone!

@sarayourfriend
Copy link
Copy Markdown
Collaborator Author

sarayourfriend commented Nov 13, 2020

Maybe it's okay from this point forward?

It doesn't seem to be. Every time I've tried to change a JS file it breaks on the test run from pre-commit. @sirreal did you notice this happening for you? I don't think this is safe to land if it's breaking the pre-commit hook.

@sirreal
Copy link
Copy Markdown
Collaborator

sirreal commented Nov 13, 2020

@sirreal did you notice this happening for you?

I had noticed this, but it didn't happen during my rebase. I'd prefer to push forward until it becomes a problem, if it's only an issue on certain commits we're probably fine. The end goal is for this project to be integrated in Gutenberg and the tooling that's having issues won't be used so I'd like to avoid investing time there.

@sarayourfriend sarayourfriend merged commit 2128b1d into master Nov 13, 2020
@sarayourfriend sarayourfriend deleted the try/incremental-ts-lang branch November 13, 2020 15:34
@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 13, 2020

The end goal is for this project to be integrated in Gutenberg and the tooling that's having issues won't be used so I'd like to avoid investing time there

I agree 👍 . I did ma best with @itsjonq/zero. This was the biggest project my scripts package has ever needed to manage 😊

@ItsJonQ ItsJonQ mentioned this pull request Nov 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sirreal sirreal sirreal approved these changes

@ItsJonQ ItsJonQ Awaiting requested review from ItsJonQ

+1 more reviewer

@v9ai v9ai v9ai left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sarayourfriend sarayourfriend

Labels

systems typescript

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sarayourfriend @ItsJonQ @sirreal @v9ai