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 assignment to readonly property to allow running in strict mode #270

Merged
merged 3 commits into from
Apr 25, 2018
Merged

fix assignment to readonly property to allow running in strict mode #270

merged 3 commits into from
Apr 25, 2018

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Apr 5, 2018

Replaced assignment to readonly Function.name with Object.defineProperty, as you can't assign to readonly properties in strict mode.

Fixes #268

…rty, as you can't assign to readonly properties in strict mode.
@G-Rath
Copy link
Contributor Author

G-Rath commented Apr 5, 2018

@broofa for some reason the readme.md was changed for me - It seems the uuids were regenerated?

Since I don't know how that file works, I've not committed the changes. Let me know if I should.

…ing to configure it.

This should allow compatibility with both strict mode and non-standard, pre-ES2015 implementations, such as in node 0.12.x
@G-Rath
Copy link
Contributor Author

G-Rath commented Apr 5, 2018

@broofa it seems this solution won't work with node 0.12.x, as it's a non-standard, pre-ES2015 implementation.

I've fixed this by adding a conditional check to make sure the name property is actually configurable before trying to configure it.

Given that prior to this the assignment to generateUUID.name wasn't apparently doing anything anyway, I don't expect this to be a breaking change.

Let me know what you think, and if there are any adjustments you'd like me to make :)

@BenjaminVanRyseghem
Copy link

I published https://www.npmjs.com/package/uuid-pull-270 on NPM based on @G-Rath branch in case someone need it as well 😄

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@broofa broofa merged commit df40e54 into uuidjs:master Apr 25, 2018
@broofa broofa mentioned this pull request Apr 25, 2018
@johngeorgewright
Copy link

Hello @broofa, any news on when this will get published? Struggling to get node-uuid working without this patch.

broofa pushed a commit that referenced this pull request Jun 22, 2018
…270)

* Replaced assignment to readonly Function.name with Object.defineProperty, as you can't assign to readonly properties in strict mode.

* added check to make sure the name property is configurable before trying to configure it.

This should allow compatibility with both strict mode and non-standard, pre-ES2015 implementations, such as in node 0.12.x

* Removed extra blank newline.
broofa pushed a commit that referenced this pull request Jun 22, 2018
…270)

* Replaced assignment to readonly Function.name with Object.defineProperty, as you can't assign to readonly properties in strict mode.

* added check to make sure the name property is configurable before trying to configure it.

This should allow compatibility with both strict mode and non-standard, pre-ES2015 implementations, such as in node 0.12.x

* Removed extra blank newline.
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