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

ESM Script base class #6367

Merged
merged 84 commits into from
Jun 18, 2024
Merged

ESM Script base class #6367

merged 84 commits into from
Jun 18, 2024

Conversation

marklundin
Copy link
Member

@marklundin marklundin commented May 14, 2024

This PR proposes a new Script base class for ESM Scripts. Attributes are declared via jsdoc tags as opposed to Script.attributes.add(). ScriptType class persists for legacy support. The new Script base class:

API Changes

  • New ESM Script Class
    A new minimal base. Attributes are declared via jsdoc tags. Runtime attributes via Script.attributes.add() is removed. Fixes ScriptType does not support class syntax #6316

  • assignRawToValue - New method that takes a script instance, a schema and value and assigns a new value on the script instance`

  • ScriptRegistry.addSchema/getSchema - Stores and fetches an attributes schema from it's associated asset <scriptName, schema>

Files renaming

For consistency, the following files have been renamed
script.js -> script-create.js - Contains legacy pc.createScript() and pc.registerScript()
script.js contains new Script base class
script-type.js contains legacy ScriptType class

Tests

npm run test:karma

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
engine ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 4:05pm

@Maksims
Copy link
Contributor

Maksims commented May 14, 2024

What would be the impact to users in terms of migrations or API changes?

@marklundin
Copy link
Member Author

What would be the impact to users in terms of migrations or API changes?

pc.createScript() will still work and return a ScriptType which should function in the same way. ESM users will use Script as a base class without any ScriptAttribute functionality (events, rawToValue deep copying)

Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

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

Approving with a final round of comments.

@marklundin marklundin merged commit fd40f29 into main Jun 18, 2024
8 checks passed
@marklundin marklundin changed the title ESM script base class ESM Script base class Sep 11, 2024
@marklundin marklundin deleted the esm-script-class branch September 11, 2024 16:32
marklundin added a commit that referenced this pull request Sep 12, 2024
@marklundin marklundin mentioned this pull request Sep 12, 2024
marklundin added a commit that referenced this pull request Sep 13, 2024
* Cherry pick -> ESM Script base class
#6367

* Ensure ScriptType members are initialized correctly (#6908)

* Fix initialization of __attributes in ScriptType class

* Refactor ScriptType class and add constructor

* Remove empty line in ScriptType class definition

* Adds @Alpha flag to Script Class

* merge fixes + update to v1 linting requirements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

ScriptType does not support class syntax [RFC] JSDoc Script Attributes
7 participants