feat: update dependencies #212
Conversation
leon19
requested review from
asyncapi-bot-eve,
jonaslagoni and
magicmatatjahu
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
| sourcemap: true, | ||
| dir: outputDir, | ||
| exports: "auto", | ||
| paths: { |
There was a problem hiding this comment.
In react@18 the production build is conditionally exported based on the value of NODE_ENV
There was a problem hiding this comment.
Also, the react/cjs/react-jsx-runtime.production.min file can no longer be resolved because the file is not part of the react package.json#exports
| // check substring of the desired error | ||
| expect((error as Error).message).toContain('Invalid hook call.'); | ||
| // A console.error() will be printed due to illegal use of hooks but only in development mode | ||
| expect(() => render(<Component />)).toThrow("Cannot read properties of null (reading 'useState')"); |
There was a problem hiding this comment.
In react@18 this no longer throws with "Invalid hook call.".
Instead it prints the error via console.error() but only in development mode
There was a problem hiding this comment.
@leon19 thanks for taking the time to provide this!
I have a question I hope you can help clarify ✌️ Since we are jumping a few major versions in some of the dependencies, notably in the react dependency. Will existing templates (written with React 17 in mind) be backward compatible with this change?
|
@jonaslagoni I tested it at work with Sadly, I have tried to set it up locally with WIll try to find out if the upgrade is possible when I have the time 👌 |
|
I would probably try with just Generator and HTML-template (skipping CLI for simplicity) and using NPM link 🤔 Definitely need that test to go through 😄 |
|
@jonaslagoni I was able to test it as you mentioned. It failed 😅 This change is needed in the template to get it to work export function renderSpec(asyncapi, params) {
loadLanguagesConfig();
const config = prepareConfiguration(params);
const stringified = stringifySpec(asyncapi);
const component = <AsyncApiComponent schema={stringified} config={config}/>;
- return ReactDOMServer.renderToString(component);
+ return ReactDOMServer.renderToString(() => component);
}The good news is that by updating only that line, the generator still works with But yes, this PR cannot be merged before applying the change in the HTML template |
leon19
force-pushed
the
update-dependencies
branch
from
2ebd138 to
b7d867f
Compare
|
Hello, @leon19! 👋🏼 |
|
|
@leon19 so looks like that even if we fix HTML template, the upgrade is a breaking change for generator that depends on react sdk |
4 participants
Description
Related issue(s)
Fixes #208