Skip to content

Conversation

@line-o
Copy link
Member

@line-o line-o commented Jun 8, 2021

  • switch to separate new templating library
  • replace calls to templates:load-source and tempates:link-to-app

@line-o line-o marked this pull request as draft June 8, 2021 13:33
@line-o line-o requested a review from wolfgangmm June 8, 2021 13:38
line-o added 3 commits June 9, 2021 21:02
- match maven workflow (unreleased is a SNAPSHOT)
- have the same version in package.json as well
@line-o line-o force-pushed the feat/new-templating branch from a9039ee to 81440de Compare June 9, 2021 19:04
@line-o line-o changed the title WIP [feat] switch to new templating library [feat] switch to new templating library Jun 9, 2021
@line-o line-o marked this pull request as ready for review June 9, 2021 19:05
@line-o line-o requested a review from joewiz June 9, 2021 19:05
<pre class="error" data-template="templates:error-description" />
<div class="source-links">
<p>View source: <a href="login.html" class="templates:load-source">this page</a>.</p>
<p>View source: <a href="login.html">this page</a>.</p>
Copy link
Member

@joewiz joewiz Jun 9, 2021

Choose a reason for hiding this comment

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

This entire div should be removed, since without the templates:load-source the link won't be transformed into the link to eXide. See the corresponding change in function-documentation and documentation.

@joewiz
Copy link
Member

joewiz commented Jun 9, 2021

@line-o I've built and deployed the PR branch locally, and it's really working well, except for the Remote Monitoring pane. The error there is due to the presence of templates:load-source in https://github.com/eXist-db/monex/blob/master/src/main/xar-resources/remotes.html#L73-L75. If you remove those calls, I think we'll be ok.

Come to think of it, I realize I'm not entirely clear why @wolfgangmm removed templates:load-source? I imagine it was for security, but the links in this case might serve a purpose, as with your workaround to the removal of templates:link-to-app at https://github.com/eXist-db/monex/pull/153/files#diff-45a2f5d1461c423366429c0529e3ec956fc4976aef96962eb4bada303b4c57b4L629.

@wolfgangmm Is there a reason we should avoid linking to eXide in monex? This happens in 2 places:

  1. Browse indexes, http://localhost:8080/exist/apps/monex/collection.html:
Screen Shot 2021-06-09 at 3 53 07 PM
  1. Remote monitoring, http://localhost:8080/exist/apps/monex/remotes.html
Screen Shot 2021-06-09 at 3 55 46 PM

@line-o
Copy link
Member Author

line-o commented Jun 9, 2021

@joewiz thanks for catching those two that I missed
I see no problems in opening files in eXide from here (as only DBAs are allowed to see monex).

@joewiz
Copy link
Member

joewiz commented Jun 9, 2021

I should add that npm run cypress flagged the problem with remotes.html, and removing the calls to templates:load-source allowed cypress to pass all tests locally.

However, on CI, cypress doesn't even get to the point of being able to flag the remotes.html issue. From https://travis-ci.com/github/eXist-db/monex/jobs/512516504:

$ npm ci
> [email protected] postinstall /home/travis/build/eXist-db/monex/node_modules/cypress
> node index.js --exec install
/home/travis/build/eXist-db/monex/node_modules/is-installed-globally/index.js:12
	} catch {
	        ^
SyntaxError: Unexpected token {
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:617:28)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/travis/build/eXist-db/monex/node_modules/cypress/lib/util.js:43:29)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] postinstall: `node index.js --exec install`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2021-06-09T19_06_14_576Z-debug.log
The command "npm ci" exited with 1.
0.43s$ npm run cypress -- --record
> [email protected] cypress /home/travis/build/eXist-db/monex
> cypress run "--record"
/home/travis/build/eXist-db/monex/node_modules/is-installed-globally/index.js:12
	} catch {
	        ^
SyntaxError: Unexpected token {
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:617:28)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/travis/build/eXist-db/monex/node_modules/cypress/lib/util.js:43:29)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] cypress: `cypress run "--record"`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] cypress script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2021-06-09T19_06_15_020Z-debug.log
The command "npm run cypress -- --record" exited with 1.

I wonder if we need to migrate from Travis to GitHub Actions, so instead of https://github.com/eXist-db/monex/blob/master/.travis.yml we'd use https://github.com/eXist-db/documentation/blob/master/.github/workflows/ci.yml.

It would be amazing if we could restore CI tests!

@line-o
Copy link
Member Author

line-o commented Jun 9, 2021

I think it is worth a try to switch to GitHub Actions. But not necessarily in this PR. :)
The node version might unsuitable to run cypress v7.

Copy link
Member

@joewiz joewiz left a comment

Choose a reason for hiding this comment

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

Works for me, and npm run cypress passes locally.

@joewiz
Copy link
Member

joewiz commented Jun 9, 2021

@line-o Looks ready to merge to me, but I see a checkbox is not yet checked in the original post. Could you confirm when it's ok to merge from your perspective?

@line-o
Copy link
Member Author

line-o commented Jun 9, 2021

Was unchecked because of your findings. Now good to merge @joewiz

@joewiz joewiz merged commit 01959b1 into eXist-db:master Jun 9, 2021
@line-o line-o deleted the feat/new-templating branch October 17, 2025 10:43
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