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

babel-plugin-inline-webgl-constants v2.0.0-alpha.1 #1867

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Dec 6, 2023

Already published under 'beta' tag -- updating local files.

When used in our local workspace, the plugin imports the local @luma.gl/constants, which means constants need to be built before using the plugin. I've worked around that issue by conditionally loading the plugin in Babel config, only when it's needed. Lerna builds in order, so we know that by the time we're building a module depending on constants, constants have already been built.

… that require it

Lerna builds `@luma.gl/constants` before building any packages that depend
on it. This plugin uses LOCAL `@luma.gl/constants`, and will fail without
it, so delay using the plugin until reaching a package depending on constants.
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

LGTM just a thought

];

// Lerna builds `@luma.gl/constants` before building any packages that depend
// on it. This plugin uses LOCAL `@luma.gl/constants`, and will fail without
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be better to just a pinned dependency on luma.gl/constants so we don't depend on the local copy?

This utility could just as well have been in its own repo, it was just a convenience to have it here, but since it seems to get involved in the overall build process that seems to create unnecessary problems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't imagine we add new GL constants often, but I suppose testing that more easily would the advantage of keeping it here? No strong preference there. With the plugin in this repo, I wasn't sure how to safely exclude it from yarn bootstrap dependency linking.

@donmccurdy donmccurdy merged commit 30c2c4b into visgl:master Dec 7, 2023
2 checks passed
@donmccurdy donmccurdy deleted the donmccurdy/babel-plugin-inline-webgl-constants-v2.0.0-alpha.1 branch December 7, 2023 17:13
@donmccurdy donmccurdy added this to the 9.0.0 milestone Feb 26, 2024
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.

2 participants