Skip to content

Add documentation about InlineJavascriptRequirement expressionLib #242

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

Merged
merged 8 commits into from
Oct 10, 2022

Conversation

kinow
Copy link
Member

@kinow kinow commented Sep 1, 2022

Closes #126

@kinow
Copy link
Member Author

kinow commented Sep 1, 2022

Started using underscore.js, but trying with their latest builds (UMD and ESM) failed JS validation in cwltool (not sure if their code is ES5 compatible?).

So today re-wrote it without underscore.js. APAC meeting now, then will re-read it later today or tomorrow morning and then it should be ready for review 👍

Comment on lines 9 to 15
* An example function that uses a function from the included
* functions.js file to create a Hello World message.
* @param {Object} message - CWL document input message
*/
var createHelloWorldMessage = function (message) {
return capitalizeWords(message);
};
Copy link
Member

Choose a reason for hiding this comment

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

Interesting to show a mixture of includes and inline; might be better to show each case separately?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't take credit for the example, I modeled it after this example common-workflow-language/common-workflow-language#788 (comment) 🙂

I was trying to make this example simpler. If we break it down in two examples, would it be for the same code?

I liked this example as it shows that whatever is loaded in an entry of expressionLib gets added to the global context and will be available for other entries of expressionLib, but also to other parts of the code where Expressions are supported.

If we break it down into two examples, I think it would be nice to show

  • just $include
  • just inline JavaScript
  • both combined

I think that section would get a bit longer, and the user would get the same idea in both cases… I think… WDYT?

@mr-c
Copy link
Member

mr-c commented Sep 2, 2022

Started using underscore.js, but trying with their latest builds (UMD and ESM) failed JS validation in cwltool (not sure if their code is ES5 compatible?).

Did you try https://github.com/common-workflow-language/common-workflow-language/blob/212e1c31107676cdbcb534b1c70257e58972df06/v1.0/v1.0/underscore.js ?

Yes, it is likely that newer versions of underscore.js use features not in ECMAScript 5.1

@kinow kinow force-pushed the expressionlib branch 2 times, most recently from 2dc9f34 to c6c7217 Compare September 2, 2022 21:18
@kinow
Copy link
Member Author

kinow commented Sep 2, 2022

Started using underscore.js, but trying with their latest builds (UMD and ESM) failed JS validation in cwltool (not sure if their code is ES5 compatible?).

Did you try https://github.com/common-workflow-language/common-workflow-language/blob/212e1c31107676cdbcb534b1c70257e58972df06/v1.0/v1.0/underscore.js ?

Ah, I didn't try that.

Yes, it is likely that newer versions of underscore.js use features not in ECMAScript 5.1

I think it's worth repeating to users that whatever JavaScript they have needs to be compatible with the version of the JS Engine used by the CWL Runner, cwltool in this case. Will add a note or paragraph about it.

@kinow
Copy link
Member Author

kinow commented Sep 2, 2022

image

I was going to end that note with "You can use a transpiler to convert newer JavaScript code down to ECMAScript 5.1.", but thought it better to leave to #245 👍

.split(' ')
.map(function (token) {
return token.charAt(0).toUpperCase() + token.slice(1);
})
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the simplest JS code I could think of, that's actually common to see in code bases (different than fibonacci sequence or factorial) as it's not provided by JavaScript out of the box as in Python with 'string'.capitalize(). Also avoided anything random, so that the output of the examples is more deterministic (which I intend to make even more deterministic by removing user name, directories, etc. with a container + Sphinx directive… for another issue).

@kinow kinow marked this pull request as ready for review September 2, 2022 22:02
@kinow kinow requested a review from mr-c September 2, 2022 22:02
@kinow
Copy link
Member Author

kinow commented Sep 24, 2022

(rebased)

@kinow
Copy link
Member Author

kinow commented Sep 24, 2022

@kinow kinow requested a review from mr-c September 24, 2022 07:32
mr-c
mr-c previously approved these changes Oct 10, 2022
@mr-c mr-c enabled auto-merge (squash) October 10, 2022 09:30
@mr-c mr-c merged commit 76dd547 into main Oct 10, 2022
@mr-c mr-c deleted the expressionlib branch October 10, 2022 09:31
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.

Advanced CWL expression JavaScript, using external libraries and expressionLib
2 participants