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

Handle relative path from web app to project root in a single place #2508

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sodic
Copy link
Contributor

@sodic sodic commented Feb 13, 2025

There are still two places of duplication left:

  • ServerGenerator/JsImport.hs
  • WebAppGenerator/JsImport.hs

But those two are too complicated to handle now (it would require rearraning many imports/modules) . I have bigger plans that will also take care of those two, so I'm leaving them alone for now.

function getWaspProjectDirAbsPathFromCwd(): string {
const webAppDirAbsPath = process.cwd()
return path.join(webAppDirAbsPath, '{= waspProjectDirFromWebAppDir =}')
function getWaspProjectDirAbsPathWhileInWebAppDir(): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this function's nam to clearly communicate its assumptions.

@@ -13,5 +14,6 @@ import { join as joinPaths } from 'path'
* to be relative to the `web-app` directory i.e. `../../../projectDirPath`.
*/
export function resolveProjectPath(path: string): string {
return joinPaths('../../../', path)
const waspProjectDirFromWebAppDir = '{= waspProjectDirFromWebAppDir =}'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I extraced a variable here to put a name on the string because, once the template is rendered, the resulting code loses the information provided by {= waspProjectDirFromWebAppDir =} and is left only with ../../../.

const webAppDirAbsPath = process.cwd()
return path.join(webAppDirAbsPath, '{= waspProjectDirFromWebAppDir =}')
function getWaspProjectDirAbsPathWhileInWebAppDir(): string {
return path.resolve(resolveProjectPath('./'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@infomiho Double check if it's ok that I import this function here.

I tested it out and it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

That helper has been used in the user's vite.config.ts because it was used from web-app. This serves the exact same purpose - it's a good use!

".env"
],
"comment-filip": "We now have to watch ../../../src/ because we're importing client files directly",
"comment-filip": "We now have to watch {= relativeUserSrcDirPath =} because we're importing client files directly",
"ext": "ts,mts,js,mjs,json"
Copy link
Contributor

Choose a reason for hiding this comment

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

@Zeko369 mentioned that the server doesn't restart on TSX file changes, should we extend this list? Does it make sense for server to restart on TSX changes since we are not rendering anything on the server?

@@ -37,7 +35,7 @@ function parsePathToUserCode(
importerPath: string
): RelativePathToUserCode | null {
const importerPathRelativeToWaspProjectDir = path.relative(
waspProjectDirAbsPath,
getWaspProjectDirAbsPathWhileInWebAppDir(),
importerPath
)
return importerPathRelativeToWaspProjectDir.startsWith('src/')
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we maybe want to template the src dir? I seem to have forgotten to use the srcDirInWaspProjectDir variable.

@sodic sodic self-assigned this Feb 18, 2025
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