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

Fix overwritten export (#36) by adding a backwards compatible dist/hashids.js fallback file #53

Merged
merged 7 commits into from
Sep 8, 2018
Merged

Fix overwritten export (#36) by adding a backwards compatible dist/hashids.js fallback file #53

merged 7 commits into from
Sep 8, 2018

Conversation

niieani
Copy link
Owner

@niieani niieani commented Aug 31, 2018

Updates babel to v7 and additionally adds support for ES modules under Node (.mjs).

added a backwards compatible dist/hashids.js fallback file for CJS to fix #36
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8385a6c on niieani:fix-overwritten-export into 44a67d5 on ivanakimov:master.

@coveralls
Copy link

coveralls commented Aug 31, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling d978bca on niieani:fix-overwritten-export into 44a67d5 on ivanakimov:master.

@jd327
Copy link
Collaborator

jd327 commented Aug 31, 2018

@niieani Thank you, this is great! Do you know what's up with https://travis-ci.org/ivanakimov/hashids.js/jobs/423206991 ?

Also, in a backwards compatible way -- does it work for frontend/backend/old/new? Anything to be aware of?

@niieani
Copy link
Owner Author

niieani commented Sep 1, 2018

Ah, it seems babel's preset-env by default targets what's compatibile with Node >= 6. I can add a browserslist setting in package.json to list the minimum target, just let me know what combination of browsers/node versions you'd like to support in the dist version.

Any node version will use the dist/index, which behaves the same way as it did in the past. The minified version for use in the browser is the same as it used to be, though we should set the minimum target and a "browser" prop in package.json (I'll do that).

Then there's support for .mjs which is consumed by Node with experimental-modules flag and by Webpack 4.

We could probably drop the 0.xx versions of Node from Travis. As for Node v4, it's up to you.

@niieani
Copy link
Owner Author

niieani commented Sep 1, 2018

I've dropped all unsupported Node versions (4 is no longer supported by the Node organization), if you deem that incorrect we can just adjust the setting in package.json and rebuild again. I've also added a "browser" prop to point to the minified version and added a "module" one for builders that support ES Modules, but don't support the .mjs extension.

@jd327
Copy link
Collaborator

jd327 commented Sep 4, 2018

@niieani Many thanks for all the changes. I guess since maintenance cycle is over for v4, we can probably drop it. But we'd have to update either major or minor version, so devs are aware of the changes. Think you could update the changelog with your updates, and then I merge?

@jd327 jd327 merged commit 72975bf into niieani:master Sep 8, 2018
@jd327
Copy link
Collaborator

jd327 commented Sep 8, 2018

Thank you @niieani. I updated the changelog since 1.1.5 was never released. Please let me know if you see something else that's funky.

@jd327
Copy link
Collaborator

jd327 commented Sep 15, 2018

@niieani does the current code look release-ready to you?

@niieani
Copy link
Owner Author

niieani commented Sep 15, 2018

@ivanakimov yeah, it looks fine.

@jd327
Copy link
Collaborator

jd327 commented Sep 15, 2018

Thanks for all the changes @niieani 👍
1.2.0 published, cc @anisabboud

@niieani niieani deleted the fix-overwritten-export branch September 15, 2018 18:23
@niieani
Copy link
Owner Author

niieani commented Sep 15, 2018

Works perfectly https://codesandbox.io/s/rmk16j6q8m
Thanks @ivanakimov!

@pierrefh
Copy link

pierrefh commented Sep 17, 2018

Hi @ivanakimov I noticed a regression in my node.js (version 8 + babel) app since 1.2.1 (was previously 1.1.4): _hashids2.default is not a constructor when I import hashids.

import Hashids  from 'hashids'
const hashids = new Hashids('some salt', 10, HASHIDS_CHARS)

When rolling back to @1.1.4 explicitly the issue is gone.

niieani pushed a commit that referenced this pull request Aug 15, 2019
Fix overwritten export (#36) by adding a backwards compatible dist/hashids.js fallback file
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