Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Ensure fs.Stats (and other classes on fs) are not mutated. #23

Merged
merged 1 commit into from Aug 19, 2019

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Aug 19, 2019

fs.Stats is a standard prototypal class (e.g. function Stats() {}), so it was being wrapped by our fs monitoring system. Unfortunately, wrapping fs.Stats in a plain function (which did return original.apply(this, arguments)) doesn't properly emulate a number of "standard" class behaviors. Specifically:

  • static methods and properties are lost
  • accessing fs.Stats.prototype does not refer to the original prototype

This last item was causing the following error when using esm:

TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function
    at $o (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:224377)
    at xu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:226413)
    at wu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227391)
    at Eu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227999)
    at Module.<anonymous> (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:295976)
    at n (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:279589)
    at getFeatures (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:5:33)
    at Object.<anonymous> (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:31:18)

This is because esm is essentially doing:

let isFile = fs.Stats.prototype.isFile;

To stash off the isFile implementation for recurring usage. The fact that when wrapped fs.Stat.prototype.isFile was undefined caused the error.

`fs.Stats` is a standard prototypal class (e.g. `function Stats() {}`),
so it was being wrapped by our `fs` monitoring system. Unfortunately,
wrapping `fs.Stats` in a plain function (which did `return
original.apply(this, arguments)`) doesn't properly emulate a number of
"standard" class behaviors. Specifically:

* static methods and properties are lost
* accessing `fs.Stats.prototype` does not refer to the original prototype

This last item was causing the following error when using `esm`:

```
TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function
    at $o (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:224377)
    at xu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:226413)
    at wu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227391)
    at Eu (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:227999)
    at Module.<anonymous> (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:295976)
    at n (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/esm/esm.js:1:279589)
    at getFeatures (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:5:33)
    at Object.<anonymous> (/home/travis/build/ember-cli/ember-cli/tmp/node-FGpJBujF.tmp/node_modules/@ember-data/-build-infra/src/features.js:31:18)
```

This is because `esm` is _essentially_ doing:

```js
let isFile = fs.Stats.prototype.isFile;
```

To stash off the `isFile` implementation for recurring usage. The fact that when wrapped `fs.Stat.prototype.isFile` was undefined caused the error.
@rwjblue rwjblue requested a review from hjdivad August 19, 2019 20:46
@hjdivad
Copy link
Collaborator

hjdivad commented Aug 19, 2019

:shipit:

@rwjblue rwjblue merged commit 578688b into heimdalljs:master Aug 19, 2019
@rwjblue rwjblue deleted the ensure-classes-not-wrapped branch August 19, 2019 20:56
@rwjblue rwjblue added the bug label Aug 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants