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

chore: repository improvements #10

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

rossipedia
Copy link
Contributor

@rossipedia rossipedia commented Jul 17, 2023

Description

Buckle up :)

  • Fixes eslint violations
  • Removes unused and empty props declarations (eslint was complaining about them)
  • Configures prettier as well as Tailwind's prettier plugin (which sorts class names consistently)
  • Applies prettier --write to the whole repo
  • Builds the final stylesheet.css automatically via Vite's postcss integration
  • Removes the need for moveStylesheet.js
  • Configures the root folder and the src/remix-app-for-testing as separate npm workspaces, so that we get proper package hoisting and deduplication of dependencies
  • Removes the need for relative path imports in remix-app-for-testing, which now imports the dev tools the same way that users will.
  • Removes the need for a separate tailwind dev process, since that is handled via Vite's postcss integration.
  • Removes the react-refresh eslint plugin, since it seemed like the codebase wasn't following that recommendation anyways (would be happy to bring it back and fix the violations if that's desired).
  • Sets up react, react-dom, and @remix-run/react as peerDependencies of the package so that users don't end up with multiple versions of those packages when integrating this one.
  • Reconfigures tsconfig.json with noEmit: true so you can typecheck the codebase from the CLI or in CI without actually building anything (since that's handled by Vite)
  • Removes the need for the postinstall step in remix-app-for-testing
  • Changes the deprecated prepublish NPM script to prepublishOnly

Type of change

Please delete options that are not relevant.

  • Repository tooling

How Has This Been Tested?

Mainly just by using the remix-app-for-testing and verifying that everything still works as intended. There's not meant to be any functional changes to the final published package, this PR mainly focuses on how that package is built.

package.json Outdated
"src/remix-app-for-testing"
],
"peerDependencies": {
"@remix-run/react": "^1.18.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be bumped down to 1.15, the package should work with all versions above it because it requires useNavigate from @remix-run/react

@@ -52,6 +52,7 @@ export default defineConfig({
["react-dom"]: "ReactDOM",
["@remix-run/react"]: "@remix-run/react",
},
assetFileNames: "stylesheet[extname]",
Copy link
Contributor

Choose a reason for hiding this comment

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

are you telling me I spent 2 days trying to figure out how to output the css file and this is all it took? 😮‍💨

Copy link
Contributor

@AlemTuzlak AlemTuzlak left a comment

Choose a reason for hiding this comment

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

Awesome stuff @rossipedia, only thing I think we should change about this PR is the peerDependency on 1.18.1

@AlemTuzlak AlemTuzlak merged commit c281c35 into forge-42:main Jul 17, 2023
@rossipedia rossipedia deleted the feat/repo-improvements branch July 17, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants