feat: compatibility with latest versions of React and Next #55

J3m5 · 2023-08-03T13:33:05Z

close #54, close #56

…n project config environments use ts file for jest config, add types file for common types, improve types

… config

…deStream data-reactroot attribute is not added on the root element since react 16 so it's no more needed to add add a root div to remove the attribute afterward improve types

…config, add prettier to eslint

return a React element instead of a string and remove div from esi:include tag improve environment checking in server.tsx

examples/express/package.json

examples/express/src/components/MyFragment.jsx

examples/express/src/pages/App.jsx

examples/express/yarn-error.log

examples/next/default.vcl

src/__tests__/server/server.test.tsx

src/server.tsx

…ror.log files to gitignore

…ate deps update readme example links

J3m5 · 2023-08-15T13:19:39Z

Everything should be ok now @dunglas

dunglas

So cool! I left some minor comments, and we'll be able to merge.

README.md

.prettierignore

examples/next/compose.yaml

examples/next/pages/index.tsx

src/withESI.tsx

README.md

…sions

J3m5 · 2023-09-06T12:12:30Z

I've applied the remaining suggestions.
I also tried to reintroduce the attribute escaping of the esi:include element but it seems that the escaping is happening 2 times.
This cause the URL to be invalid and make varnish fail to request the fragment.
I don't know exactly where the second escaping is happening, I'm currently checking it but I think this happening on the user side, react may be escaping the url attribute by itself as the tests passes with the original snapshots.

J3m5 · 2023-09-06T12:16:32Z

Found this related issue: facebook/react#1444
The source: https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/server/escapeTextForBrowser.js

J3m5 · 2023-09-06T13:20:36Z

The attributes from the generated html is indeed correctly escaped

<div>
  <h1>React ESI demo app</h1>
  <esi:include 
    alt="&quot;&#x27;&lt;&amp;Alt&gt;&#x27;&quot;" 
    src="/_fragment?fragment=MyFragment&amp;props=%7B%22greeting%22%3A%22Hello%21%22%2C%22esi%22%3A%7B%22attrs%22%3A%7B%22alt%22%3A%22%5C%22%27%3C%26Alt%3E%27%5C%22%22%7D%7D%7D&amp;sign=c7102a792fa20beeb85c02081698c855c42f3cf575503202bbed224bce793845"
  >
  </esi:include>
</div>

I think this is because we are not using renderToString or/and dangerouslySetInnerHTML anymore.
The remaining problem is that the esi:include tag is rendered as not self-closed, it's working with Varnish but we are not spec compliant.
To fix it we'll probably need to create a PR on the react repository.

dunglas · 2023-09-06T20:05:21Z

As ESI is an XML vocabulary, using the long form instead of a self-closing element is most likely spec-compliant.

J3m5 · 2023-09-07T08:20:17Z

As ESI is an XML vocabulary, using the long form instead of a self-closing element is most likely spec-compliant.

The spec says

The include element specifies a fragment for assembly and allows for two optionally specified behaviors. include is an empty element; it does not have a closing tag.

It doesn't say that it MUST NOT have a closing tag so, yes, we're probably good.

J3m5 · 2023-10-19T13:50:21Z

Hi @dunglas
I can't merge, is there anything to do on your side?

dunglas

LGTM. I made some small changes directly to your branch and granted you the commit bit @J3m5. Feel free to merge!

examples/next/default.vcl

gurnro · 2024-01-06T18:26:44Z

<esi:include src="/_fragment?fragment=MyFragment&props=%7B%22greeting%22%3A%22Hello%21%22%7D&sign=69b6cc90a6744a633354c557183e6222cba2a3eae6f402cbe8eb2f525469f122" />

This is whats output to the page in the express example so matches the spec as per your conversation previously. All tests are passing this looks great. Wish I'd have seen this PR before checking out main - doh!

Excellent work!

JH added 2 commits August 3, 2023 15:22

refactor(server.tsx): organize imports, remove some anys, adapt types

25008c6

chore: update non-critical deps

1b92401

dunglas mentioned this pull request Aug 3, 2023

fix: move prop-types to dependencies #56

Closed

JH added 7 commits August 4, 2023 15:55

test: move client and server tests in different folders with their ow…

71d5d1a

…n project config environments use ts file for jest config, add types file for common types, improve types

chore: update react packages and their types, add ts-node for jest ts…

a7e38a0

… config

refactor: use renderToPipeableStream instead of deprecated renderToNo…

c245e23

…deStream data-reactroot attribute is not added on the root element since react 16 so it's no more needed to add add a root div to remove the attribute afterward improve types

chore: update deps, add exports field in package.json, update eslint …

f011e29

…config, add prettier to eslint

fix(createIncludeElement): fix react hydration error

cdb303a

return a React element instead of a string and remove div from esi:include tag improve environment checking in server.tsx

test: adapt and fix tests

d844a66

docs(WIP): add express and next + express example

42a0e84

dunglas reviewed Aug 11, 2023

View reviewed changes

JH added 7 commits August 11, 2023 17:13

docs(examples/express): cleanup, formating, improve setup, add compose

9fba89c

docs(examples/next): formating, fixed next version

b7bd742

refactor(lib): make browser detection bundler agnostic

d888d6a

chore(lib): add missing exports in package.json, add dist and yarn-er…

49fd7a0

…ror.log files to gitignore

docs(readme): adapt examples, update and add links, add line breaks

9570f2f

refactor(lib): bring back original createIncludeElement behavior

ff9cf72

chore: update prettier and eslint config and scripts, format/fix, upd…

014e843

…ate deps update readme example links

J3m5 marked this pull request as ready for review August 15, 2023 13:19

dunglas requested changes Aug 15, 2023

View reviewed changes

JH added 2 commits August 17, 2023 18:16

refactor: review fixes

cebd391

refactor: more review fixes

2bd766d

dunglas reviewed Aug 18, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

JH added 3 commits September 5, 2023 15:44

chore: cleanup default.vcl configs and fix varnish compose images ver…

fe76b95

…sions

docs: add readme to express example, update next and main readme

569522d

docs: improve examples readme

c933f03

JH added 3 commits September 6, 2023 16:36

chore(examples): update deps

d4da2c0

refactor(lib): don't escape attributes as react do it by itself now

5fe117d

refactor(express): use default export for App and improve server html

60f53d7

dunglas mentioned this pull request Sep 28, 2023

The configuration example link is broken in the README #57

Closed

dunglas added 5 commits January 3, 2024 08:28

Fix some style issues in README.md

e587da0

Update compose.yaml

1a0179c

EOF in default.vcl

9bcb5a1

Update compose.yaml

828fbf5

Fix indentation in default.vcl

18b55ed

dunglas approved these changes Jan 3, 2024

View reviewed changes

examples/next/default.vcl Outdated Show resolved Hide resolved

dunglas mentioned this pull request Jan 3, 2024

Bumping react versions, fixing some tests, adding some documentation #60

Open

dunglas changed the title ~~Maintenance~~ feat: compatibility with latest versions of React and Next Jan 3, 2024

J3m5 merged commit 8212165 into dunglas:main Feb 26, 2024
3 checks passed

J3m5 deleted the Maintenance branch February 26, 2024 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: compatibility with latest versions of React and Next #55

feat: compatibility with latest versions of React and Next #55

J3m5 commented Aug 3, 2023 •

edited by dunglas

Loading

J3m5 commented Aug 15, 2023

dunglas left a comment

J3m5 commented Sep 6, 2023 •

edited

Loading

J3m5 commented Sep 6, 2023 •

edited

Loading

J3m5 commented Sep 6, 2023

dunglas commented Sep 6, 2023

J3m5 commented Sep 7, 2023

J3m5 commented Oct 19, 2023

dunglas left a comment

gurnro commented Jan 6, 2024 •

edited

Loading

feat: compatibility with latest versions of React and Next #55

feat: compatibility with latest versions of React and Next #55

Conversation

J3m5 commented Aug 3, 2023 • edited by dunglas Loading

J3m5 commented Aug 15, 2023

dunglas left a comment

Choose a reason for hiding this comment

J3m5 commented Sep 6, 2023 • edited Loading

J3m5 commented Sep 6, 2023 • edited Loading

J3m5 commented Sep 6, 2023

dunglas commented Sep 6, 2023

J3m5 commented Sep 7, 2023

J3m5 commented Oct 19, 2023

dunglas left a comment

Choose a reason for hiding this comment

gurnro commented Jan 6, 2024 • edited Loading

J3m5 commented Aug 3, 2023 •

edited by dunglas

Loading

J3m5 commented Sep 6, 2023 •

edited

Loading

J3m5 commented Sep 6, 2023 •

edited

Loading

gurnro commented Jan 6, 2024 •

edited

Loading