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

[license] Updated the LicenseInfo class with 2 additional checks #16404

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

michelengelen
Copy link
Member

this adds 2 additional checks to the license check.

  1. Has the setLicenseKey method been run at all
  2. Was the license key set to the LicenseInfo class

asking @mapache-salvaje specifically for a review of the error messages 🙇🏼

@michelengelen michelengelen added package: x-license Specific to @mui/x-license. enhancement This is not a bug, nor a new feature labels Jan 30, 2025
@michelengelen michelengelen self-assigned this Jan 30, 2025
@mui-bot
Copy link

mui-bot commented Jan 30, 2025

Deploy preview: https://deploy-preview-16404--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 18eb656

@michelengelen michelengelen enabled auto-merge (squash) January 30, 2025 17:07
Co-authored-by: Sam Sycamore <[email protected]>
Signed-off-by: Michel Engelen <[email protected]>
Copy link
Contributor

@mapache-salvaje mapache-salvaje left a comment

Choose a reason for hiding this comment

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

couple more little nitpicks here but otherwise I think this is good to go

packages/x-license/src/utils/licenseErrorMessageUtils.ts Outdated Show resolved Hide resolved
packages/x-license/src/utils/licenseErrorMessageUtils.ts Outdated Show resolved Hide resolved
michelengelen and others added 2 commits February 1, 2025 11:35
Comment on lines +36 to +41
if (!LicenseInfo.getHasSetLicenseKeyRun()) {
showSetLicenseKeyNotCalledError();
}
if (!LicenseInfo.getIsLicenseKeySet()) {
showLicenseKeyNotSetError();
}
Copy link
Member

@oliviertassinari oliviertassinari Feb 2, 2025

Choose a reason for hiding this comment

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

The error looks out of place, I got the impression that LICENSE_STATUS is meant for this.

Comment on lines +36 to +45
LicenseInfo.isLicenseKeySet = !!key;
LicenseInfo.hasSetLicenseKeyRun = true;
}

public static getIsLicenseKeySet(): boolean {
return LicenseInfo.isLicenseKeySet;
}

public static getHasSetLicenseKeyRun(): boolean {
return LicenseInfo.hasSetLicenseKeyRun;
Copy link
Member

Choose a reason for hiding this comment

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

We could do this with less code, no?

Suggested change
LicenseInfo.isLicenseKeySet = !!key;
LicenseInfo.hasSetLicenseKeyRun = true;
}
public static getIsLicenseKeySet(): boolean {
return LicenseInfo.isLicenseKeySet;
}
public static getHasSetLicenseKeyRun(): boolean {
return LicenseInfo.hasSetLicenseKeyRun;
}

and then

 ponyfillGlobal.__MUI_LICENSE_INFO__ = ponyfillGlobal.__MUI_LICENSE_INFO__ || {
-  key: undefined,
+  key: 'default',
 };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature package: x-license Specific to @mui/x-license.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants