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

task: Make it easier to understand loaded Embed/Embed UI #84

Merged
merged 5 commits into from
Jun 7, 2022

Conversation

luca-gr4vy
Copy link
Contributor

@luca-gr4vy luca-gr4vy commented May 30, 2022

Description: Adds the package version information to a global gr4vy.version.embed variable. If embed-ui is loaded correctly, also add gr4vy.version.embed-ui with that information (passed via the frameReady event).

Depends on: https://github.com/gr4vy/embed-ui/pull/279

Ticket: https://gr4vy.atlassian.net/browse/TA-1466

Note: Not really sure how we can pass the embed-ui version information if it doesn't even load (network error)

Screenshots(s):

Screenshot 2022-06-01 at 13 58 38

📦 Published PR as canary version: 2.11.2--canary.84.0b195a8.0

✨ Test out this PR locally via:

npm install @gr4vy/embed-react@2.11.2--canary.84.0b195a8.0
npm install @gr4vy/embed@2.11.2--canary.84.0b195a8.0
# or 
yarn add @gr4vy/embed-react@2.11.2--canary.84.0b195a8.0
yarn add @gr4vy/embed@2.11.2--canary.84.0b195a8.0

@cbetta
Copy link
Member

cbetta commented May 31, 2022

I like this approach @luca-gr4vy. I would not worry about getting the EmbedUI version if it doesn't load, that's fine.

I do agree with @douglaseggleton's point about the EmbedUI version not being a version but more of a hash. I am not even sure we can read this in any way.

@cbetta
Copy link
Member

cbetta commented May 31, 2022

@luca-gr4vy to confirm: this does not bundle the whole package.json into the frontend library, right?

@luca-gr4vy
Copy link
Contributor Author

luca-gr4vy commented May 31, 2022

I like this approach @luca-gr4vy. I would not worry about getting the EmbedUI version if it doesn't load, that's fine.

I do agree with @douglaseggleton's point about the EmbedUI version not being a version but more of a hash. I am not even sure we can read this in any way.

@douglaseggleton was suggesting we could try to somehow display that as a custom header in the static script response, like x-gr4vy-version?

the other idea I had was to write a simple node script that runs on postinstall and dumps the version + commit hash into a file or something, and read it from there

@luca-gr4vy
Copy link
Contributor Author

luca-gr4vy commented May 31, 2022

@luca-gr4vy to confirm: this does not bundle the whole package.json into the frontend library, right?

it shouldn't, thanks to the webpack approach but I'll certainly double-check

@luca-gr4vy
Copy link
Contributor Author

Updated to use process.env.npm_package_version

@luca-gr4vy
Copy link
Contributor Author

luca-gr4vy commented Jun 1, 2022

Added the current branch hash info, no need for a separate node script (done with the cat command)

Screenshot 2022-06-06 at 18 58 39

@luca-gr4vy luca-gr4vy temporarily deployed to spider-sandbox June 6, 2022 12:44 Inactive
@luca-gr4vy luca-gr4vy added release Create a release when this pr is merged internal Changes only affect the internal API labels Jun 6, 2022
@cbetta
Copy link
Member

cbetta commented Jun 6, 2022

Perfect. Love it.

@luca-gr4vy luca-gr4vy self-assigned this Jun 7, 2022
@luca-gr4vy luca-gr4vy merged commit 7f58ca8 into main Jun 7, 2022
@luca-gr4vy luca-gr4vy deleted the task/TA-1466-embed-versions branch June 7, 2022 07:21
@gr4vy-code
Copy link
Collaborator

🚀 PR was released in v2.11.2 🚀

@gr4vy-code gr4vy-code added the released Issue or pull request released label Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes only affect the internal API release Create a release when this pr is merged released Issue or pull request released
Development

Successfully merging this pull request may close these issues.

3 participants