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.

Sign up for GitHub

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

Jump to bottom

node@11 freezing constants #76

Closed
mikesamuel opened this issue Nov 2, 2018 · 1 comment
Closed

node@11 freezing constants #76

mikesamuel opened this issue Nov 2, 2018 · 1 comment

Comments

@mikesamuel
Copy link
Contributor

https://nodejs.org/en/blog/release/v11.0.0/ says

[90e8f79f65] - (SEMVER-MAJOR) constants: freeze the constants object (Bryan English) #19813

which causes problems for

node-fs-ext/fs-ext.js

Lines 139 to 144 in 7c9824f

// put constants into constants module (don't like doing this but...)
for (key in binding) {
if (/^[A-Z_]+$/.test(key) && !constants.hasOwnProperty(key)) {
constants[key] = binding[key];
}
} 

// put constants into constants module (don't like doing this but...)
for (key in binding) {
  if (/^[A-Z_]+$/.test(key) && !constants.hasOwnProperty(key)) {
    constants[key] = binding[key];
  }
}
mikesamuel added a commit to mikesamuel/node-fs-ext that referenced this issue Nov 2, 2018
@mikesamuel
Minimal patch to prevent breakage on node@11
961aaf1 
Issue baudehlo#76 explains that constants is not extensible in recent builds of node@11.

This changes the loop that adds constants from the C++ code to constants to be best effort.
@mikesamuel mikesamuel mentioned this issue Nov 2, 2018
Merged
mikesamuel added a commit to mikesamuel/attack-review-testbed that referenced this issue Nov 5, 2018
@mikesamuel
Patch fs-ext with
61d8ee5 
Travis builds fails on Node.js stable with:

```
/home/travis/build/mikesamuel/attack-review-testbed/node_modules/fs-ext/fs-ext.js:142
    constants[key] = binding[key];
                   ^
TypeError: Cannot add property SEEK_SET, object is not extensible
```

This applies the fix from baudehlo/node-fs-ext#76
on postinstall so we don't have to wait for upstream.
@davedoesdev
Copy link
Contributor

Thanks! Any chance of a new version on npm?

@ikokostya ikokostya mentioned this issue Oct 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mikesamuel @davedoesdev