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

Elimination of global shared state #4573

Closed
jansav opened this issue Dec 15, 2021 · 6 comments
Closed

Elimination of global shared state #4573

jansav opened this issue Dec 15, 2021 · 6 comments
Labels
chore

Comments

@jansav
Copy link
Contributor

jansav commented Dec 15, 2021 •
edited by Nokel81
Loading

This issue is a way to track progress with elimination of global shared state in OpenLens.

Things to consider

  • Extension API contracts should not be broken
  • Incremental refactoring (i.e. system needs to remain usable)
  • Unit tests should be added while doing this

Red flag trigger words

On encounter of any of these phrases in codebase, one is likely to be dealing with global shared state.

  • .createInstance() and .getInstance()
  • static
  • export anything-with-state
  • jest.mock
  • constructor that inits or loads something

Singleton -base-class usages

Registries

Most of the registries can be replaced with reactive solutions relatively easily. Think if this is feasible at the same time.

Stores

Other stuff

Exporting already instantiated class

Global search with RegExp: export const .* = new

Other stuff
@jansav jansav added the chore label Dec 15, 2021
@jansav jansav added this to the 5.5.0 milestone Dec 15, 2021
@Nokel81
Copy link
Collaborator

Nokel81 commented Dec 15, 2021

Exporting already instantiated class

While I really want to do this part. I don't quite see how we can without a major version bump to the extension API.

@jansav
Copy link
Contributor Author

jansav commented Dec 15, 2021

Exporting already instantiated class

While I really want to do this part. I don't quite see how we can without a major version bump to the extension API.

Lets see. I think we have some ideas, but haven’t tested them about yet.

@Iku-turso
Copy link
Contributor

Note: Added Red flag trigger words to description

Red flag trigger words

On encounter of any of these phrases in codebase, one is likely to be dealing with global shared state.

  • .createInstance() and .getInstance()
  • static
  • export anything-with-state

@Iku-turso
Copy link
Contributor

Hear hear: elimination of shared global state is the current Team Technical Focus (TTF).

@jim-docker jim-docker added the p0 label Dec 20, 2021
@Nokel81
Copy link
Collaborator

Nokel81 commented Jan 5, 2022

Should the checkmarks really be checked for PRs that are closed and not merged?

@jim-docker jim-docker mentioned this issue Jan 24, 2022
Merged
@Nokel81 Nokel81 modified the milestones: 5.5.0, 5.6.0 May 11, 2022
@jim-docker jim-docker removed the p0 label Jul 12, 2022
@jim-docker jim-docker removed this from the 5.6.0 milestone Jul 12, 2022
@Nokel81
Copy link
Collaborator

Nokel81 commented Dec 20, 2022

Closing as the bulk work is now done. Just need to remove as many uses of the legacyGlobal family of functions as possible now.

🎉

@Nokel81 Nokel81 closed this as completed Dec 20, 2022
stefcameron added a commit to Mirantis/lens-extension-cc that referenced this issue Mar 28, 2023
@stefcameron
ExtensionStore is no longer a Singleton in Lens since some 6.4.x release
e615836 
- https://github.com/lensapp/lens/blob/master/packages/core/src/extensions/extension-store.ts
- lensapp/lens#6690 ("Make base store non Singleton")
- lensapp/lens#4573 ("Elimination of global shared state")

The `createInstance()` and `getInstance()` methods had been deprecated in the Lens
Extensions API, so they have been eliminated from our code base also. But we're still
using "global shared state" Cloud and Sync store instances. It's just that Lens isn't
the one creating and holding the instance anymore, we are.
@stefcameron stefcameron mentioned this issue Mar 28, 2023
Merged
stefcameron added a commit to Mirantis/lens-extension-cc that referenced this issue Mar 28, 2023
@stefcameron
ExtensionStore is no longer a Singleton in Lens since some 6.4.x rele…
501b000 
…ase (#1359)

- https://github.com/lensapp/lens/blob/master/packages/core/src/extensions/extension-store.ts
- lensapp/lens#6690 ("Make base store non Singleton")
- lensapp/lens#4573 ("Elimination of global shared state")

The `createInstance()` and `getInstance()` methods had been deprecated in the Lens
Extensions API, so they have been eliminated from our code base also. But we're still
using "global shared state" Cloud and Sync store instances. It's just that Lens isn't
the one creating and holding the instance anymore, we are.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
chore
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jansav @Iku-turso @Nokel81 @jim-docker