Pull the static file server into the CRA proxy as well #398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JulianKniephoff
added
type:dependencies
|
This pull request has conflicts ☹ |
JulianKniephoff
added a commit
to JulianKniephoff/opencast-admin-interface
that referenced
this pull request
May 16, 2024
With the refactoring of the development servers done in opencast#398 and indirectly opencast#394, there is an opportunity to boil down the project structure a bit. This PR does nothing but move files around and make the necessary changes to keep everything working. (Fingers crossed.)
This was referenced May 16, 2024
Merged
JulianKniephoff
force-pushed
the
pull-in-static-server
branch
from
6591692 to
945e51c
Compare
JulianKniephoff
added a commit
to JulianKniephoff/opencast-admin-interface
that referenced
this pull request
May 21, 2024
With the refactoring of the development servers done in opencast#398 and indirectly opencast#394, there is an opportunity to boil down the project structure a bit. This PR does nothing but move files around and make the necessary changes to keep everything working. (Fingers crossed.)
lkiesow
reviewed
May 21, 2024
package.json
Outdated
There was a problem hiding this comment.
Removing this, we need to remove this from the CI as well:
opencast-admin-interface/.github/workflows/test.yml
Lines 20 to 22 in 34dd129
There was a problem hiding this comment.
To be fair, this wasn't there when I first created this. ;D
lkiesow
requested changes
May 21, 2024
app/src/setupProxy.js
Outdated
| res.status(201); | ||
| } | ||
|
|
||
| req.url = `/${req.method}/${req.originalUrl}`; |
There was a problem hiding this comment.
The req.originalUrl already starts with /, so we don't need one as separation:
Suggested change
| req.url = `/${req.method}/${req.originalUrl}`; | |
| req.url = `/${req.method}${req.originalUrl}`; |
There was a problem hiding this comment.
This was probably caused by trying and reverting lots of different things. Fixed now.
app/src/setupProxy.js
Outdated
| } | ||
| }; | ||
|
|
||
| const testFiles = express.static(path.join(__dirname, "../../test")); |
There was a problem hiding this comment.
The path is missing the /app:
Suggested change
| const testFiles = express.static(path.join(__dirname, "../../test")); | |
| const testFiles = express.static(path.join(__dirname, "../../test/app")); |
Without this change, running npm start will immediately end in an error.
There was a problem hiding this comment.
And this was probably missed when separating this from #399.
With this, the outer npm-project is finally obsolete. See the updated `README` for how to use it and the proxy server. I abstained from restructuring the project any further to keep the PR focused, but a PR that will do nothing but move things around is in the pipeline. Note, this is based on opencast#394.
JulianKniephoff
force-pushed
the
pull-in-static-server
branch
from
945e51c to
74b8df4
Compare
lkiesow
approved these changes
May 22, 2024
JulianKniephoff
added a commit
to JulianKniephoff/opencast-admin-interface
that referenced
this pull request
May 23, 2024
With the refactoring of the development servers done in opencast#398 and indirectly opencast#394, there is an opportunity to boil down the project structure a bit. This PR does nothing but move files around and make the necessary changes to keep everything working. (Fingers crossed.)
JulianKniephoff
added a commit
to JulianKniephoff/opencast-admin-interface
that referenced
this pull request
May 23, 2024
With the refactoring of the development servers done in opencast#398 and indirectly opencast#394, there is an opportunity to boil down the project structure a bit. This PR does nothing but move files around and make the necessary changes to keep everything working. (Fingers crossed.)
JulianKniephoff
added a commit
to JulianKniephoff/opencast-admin-interface
that referenced
this pull request
May 23, 2024
With the refactoring of the development servers done in opencast#398 and indirectly opencast#394, there is an opportunity to boil down the project structure a bit. This PR does nothing but move files around and make the necessary changes to keep everything working. (Fingers crossed.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this, the outer npm-project is finally obsolete. See the updated
READMEfor how to use it and the proxy server. I abstained from restructuring the project any further to keep the PR focused, but a PR that will do nothing but move things around is in the pipeline.Note, this is based on #394.