Skip to content

External objects should not be modified #7655

Closed
Closed
External objects should not be modified#7655
Labels
discussIssues opened for discussion and feedback.fsIssues and PRs related to file-system APIs and the fs module.

Description

@micnic
micnic
opened on Jul 11, 2016

Hey, I was playing around with Object.seal() and Object.freeze() and discovered that internally Node is trying to modify my objects and I got the following in fs.watch:

TypeError: Can't add property recursive, object is not extensible

In my opinion external objects should not be modified by Node, as I observed sometimes this rule is respected, sometimes - only partially (in case of fs.appendFile).

Do we have anywhere this rule specified?

cc @nodejs/collaborators

Activity

  1. micnic
    added
    discussIssues opened for discussion and feedback.
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    on Jul 11, 2016

  2. targos commented on Jul 11, 2016

    @targos
    targos
    on Jul 11, 2016
    Member

    I don't know of any written rule but I agree that we should not modify user-supplied objects.

  3. thefourtheye commented on Jul 11, 2016

    @thefourtheye
    thefourtheye
    on Jul 11, 2016
    Contributor

    We can solve this problem for options in #7165 by making a copy.

  4. targos commented on Jul 11, 2016

    @targos
    targos
    on Jul 11, 2016
    Member

    We can solve this problem for options in #7165 by making a copy.

    Another way is to do options = Object.create(options);

  5. micnic commented on Jul 11, 2016

    @micnic
    micnic
    on Jul 11, 2016
    ContributorAuthor

    @thefourtheye yes, it would be a good place to add a solution for the fs module, I could not find yet this issue for other core modules.

    util._extend() should be used in this case I guess

  6. thefourtheye commented on Jul 12, 2016

    @thefourtheye
    thefourtheye
    on Jul 12, 2016
    Contributor

    @micnic I used Object.assign and introduced a commit in #7165.

  7. micnic commented on Jul 12, 2016

    @micnic
    micnic
    on Jul 12, 2016
    ContributorAuthor

    @thefourtheye as I know from #7208 and #7255 Object.assign() performs slower than util._extend() and it was decided to use it until Object.assign() will have a better performance

  8. micnic
    added
    fsIssues and PRs related to file-system APIs and the fs module.
    and removed
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    on Jul 12, 2016

  9. thefourtheye commented on Jul 13, 2016

    @thefourtheye
    thefourtheye
    on Jul 13, 2016
    Contributor

    @micnic Right. I use util._extend now.

  10. thefourtheye
    closed this as completedin 7542bddon Oct 9, 2016

  11. thefourtheye commented on Oct 9, 2016

    @thefourtheye
    thefourtheye
    on Oct 9, 2016
    Contributor

    Fixed in 7542bdd

  12. jasnell
    added a commit that references this issue on Oct 10, 2016

    fs: don't alter user provided `options` object

    fe9f5bc
  13. kevinoid
    added 3 commits that reference this issue on Dec 13, 2016

    fs: don't alter user provided `options` object

    94a5f53

    fs: don't alter user provided `options` object

    44b0030

    fs: don't alter user provided `options` object

    87547bf
  14. sjarmak
    added a commit that references this issue on Jul 27, 2026

    fs: don't alter user provided `options` object

    7542bdd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    discussIssues opened for discussion and feedback.fsIssues and PRs related to file-system APIs and the fs module.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions