Skip to content

[v25.x] Revert "lib: throw from localStorage getter on missing storage path" - #60750

Merged
aduh95 merged 1 commit into
nodejs:v25.x-stagingfrom
aduh95:revert-bc
Nov 16, 2025
Merged

[v25.x] Revert "lib: throw from localStorage getter on missing storage path"#60750
aduh95 merged 1 commit into
nodejs:v25.x-stagingfrom
aduh95:revert-bc

Conversation

@aduh95

@aduh95aduh95 commented Nov 16, 2025

Copy link
Copy Markdown
Contributor

This reverts commit 4fbb1ab.

@aduh95aduh95 added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 16, 2025
@github-actions

github-actionsBot commented Nov 16, 2025

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @aduh95. Please 👍 to approve.

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. v25.x labels Nov 16, 2025
@nodejs-github-bot

nodejs-github-bot commented Nov 16, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/70207/

@aduh95aduh95 changed the title Revert "lib: throw from localStorage getter on missing storage path"[v25.x] Revert "lib: throw from localStorage getter on missing storage path"Nov 16, 2025
@aduh95aduh95 added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Nov 16, 2025
targos
targos approved these changes Nov 16, 2025
View reviewed changes

@targostargos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not on main? If we want to change the implementation it will have to happen there as well

@Renegade334Renegade334 linked an issue Nov 16, 2025 that may be closed by this pull request
Regression 25.2.0 - Cannot initialize local storage without a --localstorage-file path #60704
Closed
@aduh95

aduh95 commented Nov 16, 2025

Copy link
Copy Markdown
ContributorAuthor

Why not on main? If we want to change the implementation it will have to happen there as well

I'm not sure we want, I think we established the current behavior is spec compliant, the issue is that it's too breaking for a semver-minor update

@nodejs-github-bot

nodejs-github-bot commented Nov 16, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/70208/

@aduh95
aduh95 merged commit a4dee61 into nodejs:v25.x-stagingNov 16, 2025
83 checks passed
@aduh95

aduh95 commented Nov 16, 2025

Copy link
Copy Markdown
ContributorAuthor

Landed in a4dee61

@aduh95
aduh95 deleted the revert-bc branch November 16, 2025 19:47
@github-actionsgithub-actionsBot mentioned this pull request Nov 16, 2025
Merged
fannheyward added a commit to neoclide/coc.nvim that referenced this pull request Nov 17, 2025
@fannheyward
fix(coc): rm localstorage-file flag
11b3e12
Node.js has reverted in v25.2.1
- nodejs/node#60750
- nodejs/node#60704
@fannheywardfannheyward mentioned this pull request Nov 17, 2025
Merged
@slorber

slorber commented Nov 17, 2025•
edited
Loading

Copy link
Copy Markdown

I'm not sure we want, I think we established the current behavior is spec compliant, the issue is that it's too breaking for a semver-minor update

For v25.x, that's a good move to revert 👍

However, it remains important, even for the upcoming LTS, to roll out this change properly to the Node ecosystem. My intuition is that rolling this change as-is in Node 26 is likely to trigger the same kind of pushback as we have today, on a larger scale.

It would be helpful to gather all the community-reported breakages and to provide a clear migration path for all the identified packages and patterns.

I haven't studied the problem in depth, but I'm still unsure how to migrate this pattern, for example: vm.createContext({...global}), to get the same outcome with code that should IMHO remain relatively simple (or someone would create a clone-global-safe npm package maybe 😅 )

Since this is going to break in the next v26, it looks like a good time to help the community upgrade to the upcoming behavior ahead of time. This might also help design codemods.

@slorberslorber mentioned this pull request Nov 17, 2025
Closed
This was referenced Nov 17, 2025
Merged
Merged
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Nov 18, 2025
ECG Bot
build(deps): update node to v25.2.1
c86584b
This MR contains the following updates:
| Package | Update | Change |
|---|---|---|
| [node](https://nodejs.org) ([source](https://github.com/nodejs/node)) | patch | `25.2.0` -> `25.2.1` |
MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).
**Proposed changes to behavior should be submitted there as MRs.**
---
### Release Notes
<details>
<summary>nodejs/node (node)</summary>
### [`v25.2.1`](https://github.com/nodejs/node/releases/tag/v25.2.1): 2025-11-17, Version 25.2.1 (Current), @&#8203;aduh95
[Compare Source](nodejs/node@v25.2.0...v25.2.1)
This release reverts the spec-compliant behavior of sometimes throwing on `localStorage`
access. We received feedback that this change on an experimental API was too breaking
for a semver-minor release, so we decided to push it back for Node.js 26.0.0.
##### Commits
- \[[`ff89b7b6c7`](nodejs/node@ff89b7b6c7)] - **crypto**: ensure documented RSA-PSS saltLength default is used (Filip Skokan) [#&#8203;60662](nodejs/node#60662)
- \[[`5316b580eb`](nodejs/node@5316b580eb)] - **deps**: V8: backport [`2e4c5cf`](nodejs/node@2e4c5cf9b112) (Michaël Zasso) [#&#8203;60654](nodejs/node#60654)
- \[[`ca878bc90e`](nodejs/node@ca878bc90e)] - **doc,src,lib**: clarify experimental status of Web Storage support (Antoine du Hamel) [#&#8203;60708](nodejs/node#60708)
- \[[`a4dee613fd`](nodejs/node@a4dee613fd)] - ***Revert*** "**lib**: throw from localStorage getter on missing storage path" (Antoine du Hamel) [#&#8203;60750](nodejs/node#60750)
</details>
---
### Configuration
📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 **Automerge**: Enabled.
♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.
🔕 **Ignore**: Close this MR and you won't be reminded about this update again.
---
- [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box
---
This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNzMuMSIsInVwZGF0ZWRJblZlciI6IjQxLjE3My4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
@jnoordsijjnoordsij mentioned this pull request Nov 18, 2025
Closed
@sjinkssjinks mentioned this pull request Dec 18, 2025
Merged
7 tasks
@Christymulligan7

This comment was marked as off-topic.

Sign in to view
@Jevgens

This comment was marked as off-topic.

Sign in to view
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targostargostargos approved these changes
@cjihrigcjihrigcjihrig approved these changes
@Renegade334Renegade334Renegade334 approved these changes
@gurgundaygurgundaygurgunday approved these changes

Assignees

No one assigned

Labels

dont-land-on-v22.xPRs that should not land on the v22.x-staging branch and should not be released in v22.x.dont-land-on-v24.xPRs that should not land on the v24.x-staging branch and should not be released in v24.x.fast-trackPRs that do not need to wait for 48 hours to land.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression 25.2.0 - Cannot initialize local storage without a --localstorage-file path

9 participants

@aduh95@nodejs-github-bot@slorber@Christymulligan7@Jevgens@targos@cjihrig@Renegade334@gurgunday