maxeth / react-type-animation

A React typewriter animation based on typical with extended functionality and customization.
https://react-type-animation.netlify.app/
MIT License
391 stars 24 forks source link

fix: remove default export for typeAnimation #10

Closed jrodriguezo closed 1 year ago

jrodriguezo commented 2 years ago

I've created this PR to remove the default export because it is broken the build. I've tested this in a web development hosted in Netlify and the build failed.

But also, I think doesn't make sense when the import specified in README is as follow (new version):

import { TypeAnimation } from 'react-type-animation';

For those issues, I consider this PR as an improvement,

Let me know what do you think. Thank you!

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-type-animation ❌ Failed (Inspect) Sep 11, 2022 at 9:00PM (UTC)
maxeth commented 2 years ago

Does the current setup fail when building for you? It should work fine.

The PR fails to build because export memo(TypeAnimation); is invalid. I think we could do export const TypeAnimation = memo(TypeAnimation); but I don't really think it matters cause it's a matter of preference.

I'd much rather export default solely the TypeAnimation component from the entire package, but I couldn't manage to get it working with rollup, cjs/esm outputs and type declerations. I'd appreciate any help 🙂

jrodriguezo commented 2 years ago

Yeah, I totally agree that the export doesn't matter but it was changed and it doesn't work anymore in lowest version of the package.

Do you have an example of the error? I have tried to prepare and then react-start but doesn't work...

maxeth commented 2 years ago

[...] it was changed and it doesn't work anymore in the lowest version of the package.

What exactly doesn't work? Are you trying to build a lower version? If so, why? 😅

Do you have an example of the error?

Regarding the PR: As mentioned there's an invalid export in the PR which causes an error if that's what you mean:

Failed to compile.
--
23:00:33.735 |  
23:00:33.735 | ./components/index.js
23:00:33.735 |  Error: Parsing error: Unexpected token, expected "{"
23:00:33.736 | };
23:00:33.736 | 
23:00:33.736 |  export memo(TypeAnimation);
23:00:33.736 |         ^

But I don't think this really helps as exporting only the TypeAnimation component should be the goal

jrodriguezo commented 2 years ago

1 - Everything is working now, I have updated all. Before, I had been using a caret version and it broke the build.

2 - Yeah you are rigth, I've just tried to test this repo in local environment not the implementation (that works perfectly) to try to solve that "issue" or improve the code with exports.