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

Multiple fixes #161

Merged
merged 10 commits into from Oct 10, 2021
Merged

Multiple fixes #161

merged 10 commits into from Oct 10, 2021

Conversation

nonara
Copy link
Collaborator

@nonara nonara commented Oct 3, 2021

Issues Corrected

Extras

  • Added CI workflow to ensure pull requests and pushes get tested
  • Added workflow to automate publishing to NPM and creating GH release when version tag is pushed (ie. v4.0.0)

⚠️ NOTICE ⚠️

This can be technically considered a breaking change for some, as affording proper ESM support requires adding an exports field to package.json.

This is the proper way to handle such support, but it should be released as a new major version, regardless.

For more detail on the approach used and why it's optimal, see:

Note: If a user needs a certain export that is not exported in the main index, he or she can file an issue requesting it, and we try to accommodate

@nonara
Copy link
Collaborator Author

nonara commented Oct 3, 2021

Hi @taoqf!

Hope you're doing well. I fixed most of the open issues. I will be addressing the outstanding soon.

I will merge the changes, myself, but I need your help with one thing.

I'd like to enable automatic publishing when a version tag is pushed

This will allow me to publish releases when I fix issues.

I've added github actions for testing & publishing. It also automatically creates a release, as long as we follow Conventional Commits style commits.

With this setup, all we need to do when we have a new version is to run standard-version and it will:

  1. Create an updated changelog,
  2. Increment the version number properly (according to conventional commit title)
  3. Create a tag commit

After this, when we push that commit, github actions will create a release and publish the package to NPM.

I just need you to add an NPM token to the repository, which is very easy to setup!

What I need from you is for you to:

  1. Create a publish NPM token
  2. Add the token to this repositories secrets

When you've got that added, please let me know, and I will publish all of these fixes.

@taoqf
Copy link
Owner

taoqf commented Oct 9, 2021

@nonara Done. Thanks so much for all your work.

Added wrapper for esm over CJS module and removed nonworking separate ESM build (per advice from url below).

Note: Essentially a breaking change as `exports` package.json property prevents direct import from files in dist

see: https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1
@nonara nonara force-pushed the fixes branch 2 times, most recently from 19d5a6c to 9edd14d Compare October 10, 2021 03:10
@nonara
Copy link
Collaborator Author

nonara commented Oct 10, 2021

No problem! Happy to help!

@nonara nonara marked this pull request as ready for review October 10, 2021 04:04
@nonara nonara merged commit 3140baa into taoqf:main Oct 10, 2021
@nonara nonara deleted the fixes branch October 10, 2021 04:14
@nonara nonara mentioned this pull request Oct 19, 2021
Comment on lines -19 to +22
"prepare": "npm run build"
"--------------- ": "",
"posttest": "yarn run benchmark",
"prepare": "cd test && yarn install"
Copy link

@milahu milahu Oct 4, 2023

Choose a reason for hiding this comment

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

why remove the npm run build prepare script?
this is required for "install from git" (#125), cd test && yarn install is not

fixed in #253

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.

None yet

3 participants