Skip to content

Add optional noscript parameter with sane default#1343

Open
JamesHutchison wants to merge 5 commits intoreactive-python:mainfrom
JamesHutchison:add-noscript
Open

Add optional noscript parameter with sane default#1343
JamesHutchison wants to merge 5 commits intoreactive-python:mainfrom
JamesHutchison:add-noscript

Conversation

@JamesHutchison
Copy link
Contributor

@JamesHutchison JamesHutchison commented Mar 19, 2026

Description

This adds an optional noscript parameter that is rendered. It takes a file path. The use case is that many internet bots won't execute javascript and thus won't see any content.

Checklist

Please update this checklist as you complete each item:

  • Tests have been developed for bug fixes or new functionality.
  • The changelog has been updated, if necessary.
  • Documentation has been updated, if necessary.
  • GitHub Issues closed by this PR have been linked.

By submitting this pull request I agree that all contributions comply with this project's open source license(s).

@Archmonger
Copy link
Contributor

Rather than adding a new function parameter for one HTML element, it would be more extensible to allow the user to supply their own render_index_html function within the executor.

@JamesHutchison
Copy link
Contributor Author

I went down that path and it just seemed to make things substantially more complex because you have this juggle of the renderer taking in a ReactPy object which then forces the user to look up how the function works.

I don't see a good reason to customize the Index.html renderer. Complex pages have a different, prescribed use case, which is to not use the single page component. The renderer should, in my opinion, be batteries included and just handle common use cases, which noscript absolutely is.

@Archmonger
Copy link
Contributor

Archmonger commented Mar 19, 2026

Makes sense - How about a slight tweak: Instead of having this be specifically named for "noscript", we could tweak the name to be something like append_body_content. Additionally, may as well allow the user to provide reactpy.html.* content (that gets stringified automatically).

My though process is that the current implementation in this PR doesn't have any hard restrictions around the string content being a noscript node, so we may as well open up the verbiage to allow anything,

@JamesHutchison
Copy link
Contributor Author

JamesHutchison commented Mar 19, 2026

Are you good with punting that to an issue? This has already exceeded my timebox for this feature. If nobody is asking for it then it makes sense to just not put further effort into it. I'm only asking for noscript, because I need it, and I have an existing reference implementation that pulls it from an HTML file.

There's advantages to simply having it be an argument. Like if you are hosting this online, you NEED that noscript parameter set. It's not really optional. Everything else can be set by other means.

@JamesHutchison
Copy link
Contributor Author

JamesHutchison commented Mar 20, 2026

The argument I'm making is that:

  • noscript is a logical need for a single page web app. We should make it easy for people to solve this without convoluting it by asking for additional types or abstractions
  • We have a prescribed method for getting HTML tags into the app. If its running from within ReactPy you simply use html.tagname. If it's running as a component (i.e. what I believe you had for django) then you would go through that normal method of that framework. I don't believe the index rendering applies there either?
  • Nobody has asked for more flexibility here. I'm only asking for noscript because its necessary for web page scanners and previewers such as LinkedIn that do not use javascript to view web pages.

The question I would ask is this method of adding it matches the existing and prescribed pattern for instantiating and running the app. The version I'm used to had an options object where the header configuration went.

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