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

Load 3.0 codepen examples in documentation once more #2470

Merged

Conversation

wegry
Copy link
Contributor

@wegry wegry commented Jul 14, 2021

Reasons for making this change

The React module's Component property wasn't defined in the 3.0 UMD bundle for some reason in codepen. Use ESM @rjsf/core bundle instead.

fixes #2447

Checklist

  • I'm updating documentation
    • I've checked the rendering of the Markdown text I've added
    Screen Shot 2021-07-14 at 18 29 29
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@epicfaace
Copy link
Member

Thanks for fixing this. But does this mean that the UMD bundle no longer works? Is there a potential fix for that issue as well?

@wegry
Copy link
Contributor Author

wegry commented Jul 15, 2021

@epicfaace I don't know. I haven't used UMD consciously in years if ESM is available. I'd expect if UMD was broken, it'd get raised as an issue otherwise.

@wegry wegry force-pushed the use-esm-instead-of-umd-for-codepen-2447 branch from b05be69 to 8ad4065 Compare July 18, 2021 13:14
@wegry
Copy link
Contributor Author

wegry commented Jul 18, 2021

Alright, looks like the external change I had made to the webpack.config.js was assuming the UMD bundle would be used from a bundler/module aware system. It did not account for React/ReactDOM being an ambient global (like the non-esm CDN approach).

Tested with a local build and https://gist.github.com/wegry/d17568136713cd9bf697aca624acfe3e

Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

Great, thanks for finding the fix! :)

docs/main.js Outdated Show resolved Hide resolved
@@ -14,6 +15,9 @@ module.exports = {
libraryTarget: "umd"
},
plugins: [
new MonacoWebpackPlugin({
languages: ['json']
}),
Copy link
Contributor Author

@wegry wegry Jul 18, 2021

Choose a reason for hiding this comment

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

I don't think this plugin is usable within a library. I'm almost certain that we're just incurring a build time cost with no usable change in output.

It makes sense for it to be available within the playground and docs though.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's right. Perhaps we could split it so it's only available for the playground and docs in a separate PR though?

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 of the codepen examples/snippets in the documentation work.
2 participants