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.

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

Add props to TramOneElements for devtools #198

Merged
merged 4 commits into from
Jul 7, 2022
Merged

Add props to TramOneElements for devtools #198

merged 4 commits into from
Jul 7, 2022

Conversation

JRJurman
Copy link
Member

@JRJurman JRJurman commented Jul 5, 2022

Summary

This PR adds properties to Tram-One elements so that they can expose more details in the Properties sidebar panel in Chrome Devtools. This will eventually power the Tram-One chrome extension, which will look like the following:

image

Version Bump - Patch

This is just a patch version bump, since it should have no effect on the API, and no significant change in performance / development.

Checklist

  • PR Summary
  • PR Annotations
  • Tests (No tests, for now)
  • Version Bump

Comment on lines +80 to +84
if (process.env.NODE_ENV === 'development') {
tagResult[TRAM_TAG_NAME] = tagName;
tagResult[TRAM_TAG_PROPS] = props;
tagResult[TRAM_TAG_CHILDREN] = children;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

these properties are not otherwise on the tag, so we need to add them (but only for development).

tagName here is based on what the parent component used to add this to the registry.

Comment on lines +63 to +64
// setup key queue for global observable stores when resolving mounts
setupKeyQueue(TRAM_GLOBAL_KEY_QUEUE);
Copy link
Member Author

Choose a reason for hiding this comment

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

not a huge fan of setting up a whole new key queue here, but there really isn't a better way to associate hooks used in the render with their elements.

Comment on lines +114 to +118
// development properties, these are not guarnteed to be on the element
[TRAM_TAG_NAME]?: string;
[TRAM_TAG_PROPS]?: Props;
[TRAM_TAG_CHILDREN]?: Children;
[TRAM_TAG_GLOBAL_STORE_KEYS]?: string[];
Copy link
Member Author

Choose a reason for hiding this comment

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

even if the app is running in development mode, some properties (like props or children) could be undefined.

Comment on lines +52 to +60
// if this is development environment, save global store keys to the element
if (process.env.NODE_ENV === 'development') {
const existingNewGlobalKeys = tagResult[TRAM_TAG_GLOBAL_STORE_KEYS] || [];
const newGlobalKeys = getKeyQueue(TRAM_GLOBAL_KEY_QUEUE);

// store global store keys in the node we just built
const existingNewAndBrandNewGlobalKeys = existingNewGlobalKeys.concat(newGlobalKeys);
tagResult[TRAM_TAG_GLOBAL_STORE_KEYS] = existingNewAndBrandNewGlobalKeys;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is mostly a copy-paste of how we handle local stores, so the logic isn't super novel here.

This will hopefully be simplified (or not required) when #189 is completed.

@JRJurman JRJurman marked this pull request as ready for review July 7, 2022 16:25
Copy link
Contributor

@chtinahow chtinahow left a comment

Choose a reason for hiding this comment

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

After doing the code review, LGTM

@JRJurman JRJurman merged commit 5c9a497 into master Jul 7, 2022
@JRJurman JRJurman deleted the dev-props branch July 7, 2022 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants