First working version with code extracted from Node.js 15 #1

piranna · 2021-02-08T16:31:33Z

It also has tests, it's compatible with both Node.js and browser, and have been removed Node.js specific code, but lacks of documentation.

TrySound · 2021-02-08T17:34:36Z

IMO this should be importable module in the first place and then maybe polyfill.

piranna · 2021-02-08T17:39:25Z

IMO this should be importable module in the first place and then maybe polyfill.

Can you explain a bit more? randomUUID.js file is requirable.

TrySound · 2021-02-08T17:42:20Z

index.js is polyfill. Patching builtin objects should not be default.

piranna · 2021-02-08T17:48:44Z

I can change that, but purposse of this project has been to create a polyfill since begining, that's the reason of the browser field. But yes, polyfile filename could be renamed to node.js, for example.

TrySound · 2021-02-08T17:53:17Z

Polyfill for node + implementation for browser. The api is not part of any spec and is node specific.

piranna · 2021-02-08T18:07:06Z

The api is not part of any spec

It is :-)

TrySound · 2021-02-08T18:39:51Z

Oh, cool. Thanks! I missed this.

broofa

Polyfill for node + implementation for browser. The api is not part of any spec and is node specific.

Echoing this sentiment. This is a polyfill for Node's implementation. While that is the only implementation currently in the wild, I expect that will change fairly quickly as the WICG work progresses. Specifically:

disableEntropyCache is unlikely to be part of the spec
If/when type assertions are specified, I would expect them to be TypeErrors, not the adhoc error classes seen here.

I'm okay moving forward with this but once there's a formal standard, I would assume this module switches to whatever that is (as a breaking change), with Node either leading or following the way on that I guess.

Also, there should be browser tests as well. Maybe take a page out of the uuid module's approach? (Kind of a nuisance to do, though. @ctavan: thoughts?)

README.md

randomUUID.js

Co-authored-by: Robert Kieffer <robert@broofa.com>

…verage `uuidBuf` will be inmediatly being used at `randomFillSync()`, that will throw a `TypeError` itself since it's `undefined`

piranna · 2021-02-27T14:48:47Z

Sorry for the delay. I've reviewed and fixed your concerns, and made sure tests pass on both Node.js (down to v12) and JsDOM environments, with almost 100% tests coverage. There's an issue with jest that prevents to resolve the browser field, but it's just only a line of code, so we can control it.

Thoughts? Comments?

piranna · 2021-03-09T21:52:56Z

Any update on this? How can we move it forward?

broofa

Some nit-picky changes here, but nothing critical. Approving to allow this to move forward.

Note: A few things I'd like to see added in the near future, if possible:

code linting (preferably using uuid. I.e. eslint + uuid's .eslintrc.json)
markdown linting (preferably consistent with uuid. I.e. prettier + uuid's prettier.config.js)
CI (preferably GH Action)

README.md

__tests__/randomUUID.js

broofa · 2021-03-10T15:08:19Z

@ctavan @TrySound, You guys good with this? I know it's not as far along as uuid in terms of linting and CI testing but we can backfill those things as needed.

Also, I suggest we invite @piranna to uuidjs org once this lands.

Co-authored-by: Robert Kieffer <robert@broofa.com>

piranna · 2021-03-14T15:15:40Z

Added linting fixes based on uuidjs/uuid project config files. Also applied all suggestions of @broofa.

broofa

It looks likeeslint-plugin-import, eslint-plugin-node,eslint-plugin-promise, and prettier all need to be added as devDependencies.

(Couldn't get npm test to work until I installed these.)

broofa · 2021-03-14T19:08:23Z

Oh, we should probably also go ahead and add "sideEffects": true to package.json. From https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

A "side effect" is defined as code that performs a special behavior when imported, other than exposing one or more exports. An example of this are polyfills, which affect the global scope and usually do not provide an export.

This is mostly relevent for ESM imports, which aren't supported here yet, but there's no harm in being explicit about this now.

piranna · 2021-03-14T20:40:51Z

It looks likeeslint-plugin-import, eslint-plugin-node,eslint-plugin-promise, and prettier all need to be added as devDependencies.

(Couldn't get npm test to work until I installed these.)

In my machine I've not needed to add them...

Ubuntu Mate 20.10
Node.js v15.11.0
npm 7.6.0

piranna · 2021-03-14T20:50:45Z

we should probably also go ahead and add "sideEffects": true to package.json

Seems to be really interesting :-)

broofa · 2021-03-14T22:40:39Z

In my machine I've not needed to add them...

Do you have them installed globally?

piranna · 2021-03-14T23:54:23Z

Do you have them installed globally?

No, my globals are only these ones:

[piranna@PRO2:~/github/randomUUID]
 (main) > npm ls -g
/usr/lib
├── npm-check-updates@10.2.2
├── npm@7.6.0
├── react-native-cli@2.0.1
└── sloc@0.2.1

broofa · 2021-03-15T15:39:43Z

No, my globals are only these ones:

Here's my recipe for recreating the missing modules issue...

$ node --version
v14.15.4

$ npm --version
6.14.10

$ pwd
/Users/kieffer/repos/randomUUID

$ git status
On branch piranna-main
nothing to commit, working tree clean

$ rm -fr ./node_modules
...
$ npm install
...
$ npm test
...
Oops! Something went wrong! :(

ESLint: 7.22.0

ESLint couldn't find the plugin "eslint-plugin-import".

etc...

piranna · 2021-03-15T15:53:05Z

I think npm v7 is installing missing peerDependencies... could that be the reason?

piranna · 2021-03-15T16:11:00Z

Ok, I've checked it with Node.js v14.16.0 and npm v6.14.11 and I'm seeing the same problem as you, so seems it's the thing about npm 7 installing peerDependencies. I'm going to add the missing dependencies.

piranna · 2021-03-15T16:20:31Z

I've added the missing dependencies.

broofa · 2021-03-23T23:37:33Z

My apologies for the delay. I've gone ahead and landed this since, as previously noted, I think we're at a place where we can push this out and deal with issues as/how they get reported.

I've also taken the following actions (@ctavan, take note):

Invited @piranna to the uuidjs org as a "member"
npm owner add piranna (... but piranna is not appearing yet under npm owner ls. Assuming that's because he needs to accept the invitation?)
Added randomUUID to the uuid team (w/in this org)
Enabled the new github "discussions" feature for the uuid team

@piranna: I'll leave it to you to do the npm publish honors.

broofa · 2021-03-23T23:53:51Z

Also, thanks for your work and, most of all, your patience. :-)

piranna · 2021-03-23T23:55:22Z

My apologies for the delay.

No worries :-)

Invited @piranna to the uuidjs org as a "member"

Thank you so much! :-D

npm owner add piranna (... but piranna is not appearing yet under npm owner ls. Assuming that's because he needs to accept the invitation?)

I've not received any invitation, just only the one for Github org... Maybe there's some missing step? On https://www.npmjs.com/org/uuid it shows nothing too, but I appear in https://www.npmjs.com/package/randomuuid.

@piranna: I'll leave it to you to do the npm publish honors.

403 Forbidden - PUT https://registry.npmjs.org/randomuuid - You cannot publish over the previously published versions: 0.1.0-alpha.0.

What version should I use? 0.1.0?

piranna · 2021-03-23T23:57:18Z

Also, thanks for your work and, most of all, your patience. :-)

You are welcome, I'm glad to help here :-)

broofa · 2021-03-24T00:03:41Z

403 Forbidden - PUT https://registry.npmjs.org/randomuuid - You cannot publish over the previously published versions: 0.1.0-alpha.0.

What version should I use? 0.1.0?

I'd go with 1.0.0. Minor-bump it once the ESM support gets rolled in.

Edit: npm owner ls is showing you as an owner now, so I think we're good there. Probably just a caching delay of some sort.

piranna · 2021-03-24T08:52:11Z

And... published :-)

ctavan · 2021-03-25T20:52:01Z

Woah, amazing work @piranna (and @broofa @TrySound)!

I'm sorry for the radio silence from my end. Started a new job recently so my priorities have shifted a bit. I'm confident that I'll be able to contribute again once I've onboarded :).

piranna · 2021-03-26T05:06:08Z

You are welcome :-)

piranna added 3 commits February 8, 2021 14:05

First working version with code extracted from Node.js 15

c02166b

Apply polyfill to crypto.webcrypto object

7187269

Replaced Node.js specific code

77663fa

piranna mentioned this pull request Feb 8, 2021

Support native randomUUID() uuidjs/uuid#554

Closed

broofa requested changes Feb 9, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

randomUUID.js Show resolved Hide resolved

piranna and others added 11 commits February 27, 2021 08:38

Update README.md

da791c5

Co-authored-by: Robert Kieffer <robert@broofa.com>

Update randomUUID.js

f9ea277

Co-authored-by: Robert Kieffer <robert@broofa.com>

Updated docs

11271b5

Renamed index.js to node.js

fd230bb

Use standard TypeErrors

fb3b0aa

Tests for both node and jsdom Jest testEnvironments

c5f8765

Tests for both polyfills and main implementation

f38a17d

Remove explicit checks of 'Out of memory' TypeErrors to get 100% co…

3f00140

…verage `uuidBuf` will be inmediatly being used at `randomFillSync()`, that will throw a `TypeError` itself since it's `undefined`

Fix tests in jsdom environment

718feda

Ensure it works with Node.js v12 (v10 gave errors on jest)

477c102

Clean-up

0f427b9

broofa approved these changes Mar 10, 2021

View reviewed changes

piranna and others added 3 commits March 14, 2021 13:23

Update README.md

1c63802

Co-authored-by: Robert Kieffer <robert@broofa.com>

Update README.md

30df9c0

Co-authored-by: Robert Kieffer <robert@broofa.com>

Update README.md

e62ae12

Co-authored-by: Robert Kieffer <robert@broofa.com>

piranna added 2 commits March 14, 2021 13:38

Set link for ESM modules status issue.

e618922

Enable linting

c533d73

Apply linting

7ce268c

piranna force-pushed the main branch from 1f50f87 to 7ce268c Compare March 14, 2021 15:19

piranna added 4 commits March 14, 2021 16:50

Use JSON config files instead of Javascript ones

a569cbd

Don't check linting of tests coverage autogenerated files

bffa109

Fix execution of tests on jsdom emulated browser environment

43f024f

Added .npmignore file

9baeb0e

broofa requested changes Mar 14, 2021

View reviewed changes

Add compatibility with CI using Node.js v14 and npm v6

7563ec3

broofa approved these changes Mar 23, 2021

View reviewed changes

broofa merged commit 8b7d247 into uuidjs:main Mar 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First working version with code extracted from Node.js 15 #1

First working version with code extracted from Node.js 15 #1

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

broofa left a comment

piranna commented Feb 27, 2021

piranna commented Mar 9, 2021

broofa left a comment

broofa commented Mar 10, 2021

piranna commented Mar 14, 2021

broofa left a comment

broofa commented Mar 14, 2021

piranna commented Mar 14, 2021

piranna commented Mar 14, 2021

broofa commented Mar 14, 2021

piranna commented Mar 14, 2021

broofa commented Mar 15, 2021 •

edited

piranna commented Mar 15, 2021

piranna commented Mar 15, 2021

piranna commented Mar 15, 2021

broofa commented Mar 23, 2021

broofa commented Mar 23, 2021

piranna commented Mar 23, 2021

piranna commented Mar 23, 2021

broofa commented Mar 24, 2021 •

edited

piranna commented Mar 24, 2021

ctavan commented Mar 25, 2021

piranna commented Mar 26, 2021

First working version with code extracted from Node.js 15 #1

First working version with code extracted from Node.js 15 #1

Conversation

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

piranna commented Feb 8, 2021

TrySound commented Feb 8, 2021

broofa left a comment

Choose a reason for hiding this comment

piranna commented Feb 27, 2021

piranna commented Mar 9, 2021

broofa left a comment

Choose a reason for hiding this comment

broofa commented Mar 10, 2021

piranna commented Mar 14, 2021

broofa left a comment

Choose a reason for hiding this comment

broofa commented Mar 14, 2021

piranna commented Mar 14, 2021

piranna commented Mar 14, 2021

broofa commented Mar 14, 2021

piranna commented Mar 14, 2021

broofa commented Mar 15, 2021 • edited

piranna commented Mar 15, 2021

piranna commented Mar 15, 2021

piranna commented Mar 15, 2021

broofa commented Mar 23, 2021

broofa commented Mar 23, 2021

piranna commented Mar 23, 2021

piranna commented Mar 23, 2021

broofa commented Mar 24, 2021 • edited

piranna commented Mar 24, 2021

ctavan commented Mar 25, 2021

piranna commented Mar 26, 2021

broofa commented Mar 15, 2021 •

edited

broofa commented Mar 24, 2021 •

edited