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

feat: extract GitHub into the sync-providers package #3092

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

feat: extract GitHub into the sync-providers package #3092

wants to merge 5 commits into from

Conversation

cuserox
Copy link
Contributor

@cuserox cuserox commented Aug 21, 2024 •
edited
Loading

Why does this PR exist?

Closes 1 task of #3000

In an effort to reuse the logic behind remote token storage solutions, this aims to set a starting point with GitHub, the most popular one.

What does this pull request do?

⚠️ Note how the base branch is dx

See the README for a detailed list of changes.

Testing this change

  • Run the plugin locally
  • Setup GitHub as your remote provider
  • Interact with your tokens by doing the usual actions:
    • Pull tokens
    • Push tokens
    • Add new GitHub credentials
    • Create a new branch

Additional Notes (if any)

Related follow-up tasks, will most likely need refinement depending on speific future needs:

@cuserox cuserox self-assigned this Aug 21, 2024
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 21, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: bb5ff11

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 21, 2024 •
edited
Loading

⤵️ 📦 ✨ The artifact was successfully created! Want to test it? Download it here 👀 🎁

@macintoshhelper @six7 @cuserox
Browser plugin preview (#2748)
c96985e 
* [WIP] webpack config setup for fast refresh + websocket server

* add prod/dev hmr webpack config option

* render immediately and handle AsyncMessage in startup app hook

* forward ui AsyncMessages to browser via WebSockets

* null check sentry transaction to fix browser error

* refactor AsyncMessageChannel code for browser implementation

* webpack use swc-loader for browser version + speed/bundle size plugin options

* prettify browser dev preview UI

* enable loading screen if startup params missing (for web serve + disconnected browser dev preview)

* attempt to fix webpack build for tests

* add SpeedMeasurePlugin package

* create AsyncMessageChannel dev docs

* replace web-preview.md ASCII data flow diagram with mermaid

* use radii/spacing tokens instead of px for web preview.tsx styles

Co-authored-by: Jan Six <six.jan@gmail.com>

* remove commented out startup handler (handled in startup.tsx useEffect now)

Co-authored-by: Jan Six <six.jan@gmail.com>

* remove commented out code

Co-authored-by: Jan Six <six.jan@gmail.com>

* replace px values with tokens

Co-authored-by: Jan Six <six.jan@gmail.com>

* conditional export for AsyncMessageChannel preview env

* add browser preview WEBSOCKETS_PORT env

* fix typescript issue with PreviewAsyncMessageChannel.isWsConnected

* add test coverage for AsyncMessageChannelPreview

* Browser preview debug UI (#2803)

* fix AsyncMessageChannelPreview undefined error + export WS URI

* browser preview CSS file for UI fixes

* create previewUtils for browser color scheme + setFigmaBrowserTheme

* browser preview URL params + fullscreen/theme/action modes

* two bug fixes for browser/plugin websocket preview bridge

* add preview dist folder for web preview builds

* [WIP] browser preview dev knowledge docs

* feat(dev): request startup on browser preview page open

* refactor(dev): use env vars for browser preview ws src

* fix(debug): remove console.log from asyncmessagechannelpreview

* fix(css): figmaloading full height css for browser preview

* refactor(dev): use enums for websockets src in browser preview tsx

* fix(dev): remove comments

* refactor: reuse htmlClassList variable

* remove unused package

---------

Co-authored-by: macintoshhelper <6757532+macintoshhelper@users.noreply.github.com>

---------

Co-authored-by: macintoshhelper <6757532+macintoshhelper@users.noreply.github.com>
Co-authored-by: Jan Six <six.jan@gmail.com>
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch from b7ca311 to 62b14f8 Compare August 22, 2024 09:51
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 22, 2024
View reviewed changes
packages/tokens-studio-for-figma/src/AsyncMessageChannelPreview.ts
return temp;
}
} catch (err) {
console.warn('not a valid message', err);

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
packages/tokens-studio-for-figma/src/AsyncMessageChannelPreview.ts
Comment on lines +231 to +235
if (this.environment === Environment.BROWSER) {
this.sendMessageFromBrowser({ id: msg.id, message: payload, src: WebSocketsSource.browser });
} else {
sendMessageToController({ id: msg.id, message: payload });
}

Check failure

Code scanning / ESLint

disallow `if` statements as the only statement in `else` blocks

Unexpected if as the only statement in an else block.
packages/tokens-studio-for-figma/src/AsyncMessageChannelPreview.ts
}
}
} catch (err) {
console.error(err);

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
packages/tokens-studio-for-figma/src/app/asyncMessageHandlers/startup.tsx
import FigmaLoading from '../components/FigmaLoading';

// eslint-disable-next-line
const PREVIEW_ENV = process.env.PREVIEW_ENV;

Check failure

Code scanning / ESLint

require destructuring from arrays and/or objects

Use object destructuring.
packages/tokens-studio-for-figma/src/app/components/Initiator.tsx
return temp;
}
} catch (err) {
console.error('not a valid message', err);

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
packages/tokens-studio-for-figma/src/app/preview/preview.tsx
const handleJsonEditChange = useCallback((val) => {
try {
// eslint-disable-next-line
const a = JSON.parse(val);

Check failure

Code scanning / ESLint

Disallow unused variables

'a' is assigned a value but never used.
packages/tokens-studio-for-figma/src/app/preview/preview.tsx
if (tab) {
dispatch.uiState.setActiveTab(Tabs[tab]);
}
}, []);

Check warning

Code scanning / ESLint

verifies the list of dependencies for Hooks like useEffect and similar

React Hook useEffect has missing dependencies: 'action', 'dispatch.uiState', 'subAction', 'tab', 'theme', and 'updateHash'. Either include them or remove the dependency array.
packages/tokens-studio-for-figma/src/app/preview/preview.tsx

const onThemeSelected = useCallback((type) => () => {
setFigmaBrowserTheme(type, updateHash);
}, []);

Check warning

Code scanning / ESLint

verifies the list of dependencies for Hooks like useEffect and similar

React Hook useCallback has a missing dependency: 'updateHash'. Either include it or remove the dependency array.
packages/tokens-studio-for-figma/src/app/preview/preview.tsx
}
}
},
[],

Check warning

Code scanning / ESLint

verifies the list of dependencies for Hooks like useEffect and similar

React Hook React.useCallback has a missing dependency: 'updateHash'. Either include it or remove the dependency array.
packages/tokens-studio-for-figma/src/app/preview/preview.tsx
className="content scroll-container"
>
{
[{ type: 'STARTUP' }, { type: 'CUSTOM' }].map((action) => (

Check warning

Code scanning / ESLint

Disallow variable declarations from shadowing variables declared in the outer scope

'action' is already declared in the upper scope on line 297 column 12.
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch from 62b14f8 to 98ae669 Compare August 28, 2024 12:26
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 28, 2024
View reviewed changes
packages/tokens-studio-for-figma/src/app/components/SyncSettings.tsx Outdated
@@ -117,6 +82,8 @@
setShowCreateStorageModalVisible(false);
}, []);

console.log('********** SyncSettings **********', { apiProviders, localApiState });

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch 3 times, most recently from e46e704 to 831f8d0 Compare August 29, 2024 13:57
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 29, 2024
View reviewed changes
packages/tokens-studio-for-figma/src/app/components/SyncSettings.tsx Outdated
} from '@tokens-studio/ui';
import { LIST_OF_PROVIDERS, AVAILABLE_PROVIDERS } from '@sync-providers/constants';

Check failure

Code scanning / ESLint

Disallow unused variables

'AVAILABLE_PROVIDERS' is defined but never used.
@cuserox cuserox changed the base branch from main to dx August 29, 2024 15:02
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch 7 times, most recently from cac34d0 to 55f3fa6 Compare September 3, 2024 14:58
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch 2 times, most recently from b2e5356 to 9850135 Compare September 3, 2024 15:34
@cuserox cuserox changed the title Feat/3000 extract sync providers feat: extract GitHub into the sync-providers package Sep 3, 2024
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch from 9850135 to c88367c Compare September 4, 2024 09:20
@cuserox cuserox force-pushed the feat/3000-extract-sync-providers branch from c88367c to bb5ff11 Compare September 5, 2024 12:14
Base automatically changed from dx to main October 24, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cuserox cuserox

Labels
sync github
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cuserox @rbosker @macintoshhelper