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

✨[Developer extension] npm setup override support #2304

Merged
merged 10 commits into from Jan 17, 2024
Merged

✨[Developer extension] npm setup override support #2304

merged 10 commits into from Jan 17, 2024

Conversation

amortemousque
Copy link
Contributor

@amortemousque amortemousque commented Jun 23, 2023 •
edited

Motivation

  • Allow to override the page SDKs bundles with dev bundles:
    • Inject dev bundles
    • Override page SDK instance method with the dev ones
  • Allow to override the page SDK configuration

Changes

  • Add an extension setting config to inject dev bundles
    image
  • Allow to edit the config in the extension
Click on ✏️ Edit
image image
  • Sync the extension config storage with the page session storage to be able to get the config synchronously in the main content script
  • In content-scripts/main
    • Inject the dev bundles synchronously according to the config
    • Override the SDK init configuration according to the config

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@amortemousque amortemousque changed the title test Developer extension npm dev bundle override Jun 23, 2023
@amortemousque amortemousque force-pushed the aymeric/debug-npm-setup branch 6 times, most recently from 0f1791e to b09858d Compare December 4, 2023 09:30
@amortemousque amortemousque marked this pull request as ready for review December 4, 2023 09:59
@amortemousque amortemousque requested a review from a team as a code owner December 4, 2023 09:59
@amortemousque amortemousque changed the title Developer extension npm dev bundle override [Developer extension] npm setup override support Dec 4, 2023
@amortemousque amortemousque changed the title [Developer extension] npm setup override support ✨[Developer extension] npm setup override support Dec 4, 2023
bcaudan
bcaudan reviewed Dec 4, 2023
View reviewed changes
scripts/dev-server.js Show resolved Hide resolved
developer-extension/src/common/constants.ts Outdated Show resolved Hide resolved
developer-extension/src/content-scripts/main.ts Outdated Show resolved Hide resolved
developer-extension/src/panel/components/tabs/infosTab.tsx Outdated Show resolved Hide resolved
developer-extension/src/panel/components/tabs/infosTab.tsx Outdated Show resolved Hide resolved
@bcaudan
Copy link
Collaborator

bcaudan commented Dec 12, 2023

🐛 when editing the configuration and removing a property, the removal is not applied.

bcaudan
bcaudan approved these changes Jan 11, 2024
View reviewed changes
Copy link
Collaborator

@bcaudan bcaudan left a comment

Choose a reason for hiding this comment

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

LGTM

BenoitZugmeyer
BenoitZugmeyer reviewed Jan 11, 2024
View reviewed changes
developer-extension/src/content-scripts/main.ts Outdated Show resolved Hide resolved
developer-extension/src/content-scripts/main.ts Outdated Show resolved Hide resolved
developer-extension/src/panel/hooks/useSettings.ts Outdated Show resolved Hide resolved
developer-extension/src/panel/components/tabs/settingsTab.tsx Outdated
Comment on lines 50 to 51
Overrides CDN bundles with local development bundles served by the Browser SDK development server. To
start the development server, run <Code>yarn dev</Code> in the Browser SDK root folder.
Copy link
Member

Choose a reason for hiding this comment

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

💭 thought: Having two options to override the SDK might confuse the extension user. Maybe we could clarify which setting should be used in which situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the UI. let me know what you think?
image

@amortemousque amortemousque merged commit 4308cbb into main Jan 17, 2024
17 checks passed
@amortemousque amortemousque deleted the aymeric/debug-npm-setup branch January 17, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenoitZugmeyer BenoitZugmeyer BenoitZugmeyer left review comments

@bcaudan bcaudan bcaudan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@amortemousque @bcaudan @BenoitZugmeyer