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

Support node #118

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

Support node #118

wants to merge 8 commits into from

Conversation

SeanSobey
Copy link

Thanks for contributing.

Description

Changed existing code to support and test for NodeJS usability.

Testing

Added a unit test and update the script running infrastructure to support Windows.

@SeanSobey SeanSobey mentioned this pull request Jan 6, 2022
"package": "npm run build && node scripts/create-packages.js",
"lint": "eslint docs/**/*.html src/**/*.js test/**/*.js",
"lint-types": "eslint \"types/**/*.ts\" && tsc -p types/tests/",
"test": "npm run test-types && npm run test-unit",
"test-unit": "karma start --single-run --coverage --grep",
"test-unit:dev": "karma start --auto-watch --grep",
"test-types": "tsc -p types/tests/"
"test-types": "tsc -p types/tests/",
"test-node": "node test/node.js"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why have a separate entry point for this? can we just make this a normal test in test/specs like the others?

Copy link
Author

Choose a reason for hiding this comment

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

The test needed to run on Node not via karma on the browser and this was the simplest way I could think of doing it

Copy link
Author

Choose a reason for hiding this comment

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

Also, not sure why CI is failing at finding file ./coverage/chrome/lcov.info, is this working in master? I did change some code in that space but a brief review of the changes seem to indicate that it should not change this setting.

@skywalk1411
Copy link

How do I install this package in nodejs? I try the gulp thing to no avail, anyone in here can hook me up? @SeanSobey ?

@santam85
Copy link
Collaborator

@SeanSobey is this PR still relevant?

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.

4 participants