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

feat: backport allowing noModule attribute on script tags to React 15 #19631

Closed
wants to merge 1 commit into from
Closed

Conversation

snyamathi
Copy link

Summary

This PR backports this PR which added support for the nomodule attribute to React 15 as well. I'm not sure how likely this is to get into a release, but we're still stuck on React 15 for a variety of reasons. It would be great if we could add support for this attribute to React 15.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-nomodule

nomodule
This Boolean attribute is set to indicate that the script should not be executed in browsers that support ES2015 modules — in effect, this can be used to serve fallback scripts to older browsers that do not support modular JavaScript code.

Test Plan

The tests pass locally, though I didn't add a new test since it seems like we haven't added one for each attribute (example for the PR which added nonce here) though I could look into that if need be.

@facebook-github-bot
Copy link

Hi @snyamathi!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@snyamathi snyamathi changed the title feat: allow noModule attribute on script tags feat: backport allow noModule attribute on script tags to React 15 Aug 18, 2020
@snyamathi snyamathi changed the title feat: backport allow noModule attribute on script tags to React 15 feat: backport allowing noModule attribute on script tags to React 15 Aug 18, 2020
@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@gaearon
Copy link
Collaborator

gaearon commented Sep 18, 2020

What's stopping you from upgrading?

@snyamathi
Copy link
Author

Outside of the usual tech debt, the largest limitation is #11189 - we do a lot of SSR and there's legacy code that relies on the checksum

@stale
Copy link

stale bot commented Dec 25, 2020

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@snyamathi
Copy link
Author

I would still like to see this go in if possible

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Dec 28, 2020
@stale
Copy link

stale bot commented Jun 11, 2021

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jun 11, 2021
@snyamathi
Copy link
Author

I'd still like to get this in, if possible.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jun 11, 2021
@snyamathi snyamathi closed this Nov 6, 2021
@snyamathi snyamathi deleted the nomodule branch November 6, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants