react-financial / react-financial-charts

Charts dedicated to finance.
https://react-financial.github.io/react-financial-charts/
MIT License
1.21k stars 214 forks source link

Run ndc codemod to improve next.js compatibility #522

Open rondonjon opened 3 years ago

rondonjon commented 3 years ago

I'm submitting a...

What is the current behavior

I am using react-fincancial-charts in next.js, and the compiler raises warnings about the anonymous function exports from the react-financial-charts files, e.g.

../../node_modules/@react-financial-charts/indicators/lib/indicator/tma.js
Anonymous function declarations cause Fast Refresh to not preserve local component state.
Please add a name to your function, for example:

Before
export default function () { /* ... */ }

After
export default function Named() { /* ... */ }

A codemod is available to fix the most common cases: https://nextjs.link/codemod-ndc

What is the expected behavior

Named default exports would improve the compatibility of the library with next.js applications.

Please tell us about your environment

Other information

The codemod would solve the problem by assigning a generated name (based on the filename) to all default exports that were previously anonymous.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

huydhoang commented 3 years ago

@rondonjon Hi! How did you integrate react-financial-charts with Next.js? I was having this error after installing

JSX element type 'StockChart' does not have any construct or call signatures.ts(2604)

Webpack5 couldn't compile and was complaining about the module not using ES import.

@markmcdowell Would you please add some pointers?

Platform: Node: 12 Next.js: 10.2.3 TS target: ES2019

rondonjon commented 3 years ago

@huydhoang, have you included the library in your webpack config so that it gets transpiled? https://stackoverflow.com/questions/54043498/how-to-transpile-node-modules-modules-with-babel-loader

Stack Overflow
How to transpile node_modules modules with babel-loader?
Problem: I want to build bundle files for a website for older browsers (>= IE10). My transpiled code throws errors on old Internet Explorer 11 when I transpile the code with babel 7.x using babel-
huydhoang commented 3 years ago

Thanks @rondonjon that explains. I haven't touched webpack config. Does that mean react-financial-charts only supports JS version <= ES5 directly without transpiling? I'm having a hard time understanding why a TypeScript-ready module would not be compatible with Next.js directly. Perhaps it's my lack of knowledge for how Next works? Please bear with me.

markmcdowell commented 3 years ago

Hi no it’s the other way around, you need Babel to support older versions of JavaScript. Next.js doesn’t support es modules out of the box

note we target es2017 in our typescript config

huydhoang commented 3 years ago

@markmcdowell Now that I see. Thank you for pointing out my ignorance. Looking around the codebase and existing issue tickets, I still couldn't find a way to resolve import errors within create-react-app. Will file an issue if needed.

huydhoang commented 3 years ago

Good news to folks who wanna enjoy TypeScript in React, I was able to import stuff using Aleph.js - a React framework inspired by Next in the Deno ecosystem. And since Deno compiles TypeScript directly without using Babel, no hassle in terms of JS versioning. Importing can be done through esm.sh https://esm.sh/react-financial-charts

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.