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

Make yarn-run-dev not move queries.json to build folder #162

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

simonvbrae
Copy link

@simonvbrae simonvbrae commented Sep 19, 2024

This is a draft PR, I am still making changes.

Calling yarn run dev would result in the queries.json file being moved to build/ after generation resulting in missing data sources and queries in the development server.
This PR changes lets that move happen outside of webpack so it doesn't happen when running yarn run dev, only when running yarn run build.

package.json Outdated
@@ -51,7 +51,7 @@
],
"scripts": {
"lint": "eslint src/*.js",
"dev-prod": "comunica-compile-config config/config-default.json > .tmp-comunica-engine.js && ./bin/queries-to-json.js && webpack serve --config webpack.config.js --mode production",
"dev-prod": "comunica-compile-config config/config-default.json > .tmp-comunica-engine.js && ./bin/queries-to-json.js && webpack serve --config webpack.config.dev.js --mode production",
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is correct. AFAICS, webpack.config.dev.js does not exist?

Copy link
Author

Choose a reason for hiding this comment

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

Forgot to commit my change, fixed now!

Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Thanks @simonvbrae, looks good!

Can you confirm build and serving works correctly in these cases? (because the CI currently only checks one of these)

  • yarn run dev (yarn run dev-prod should be the same)
  • yarn run build
  • node bin/generate.js

If yes, I'll merge this right away!

rubensworks added a commit that referenced this pull request Oct 15, 2024
This reverts commit 165f3fb.

Reverted because the package deployed to npm was broken because of this.
A proper fix will land in #162.
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