Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate bundle process from the source process for better accessability #1528

Open
dvkam opened this issue Apr 10, 2022 · 16 comments
Open

Separate bundle process from the source process for better accessability #1528

dvkam opened this issue Apr 10, 2022 · 16 comments
Assignees

Comments

@dvkam
Copy link
Contributor

dvkam commented Apr 10, 2022

Is your feature request related to a problem and use case? Please describe.
The Problem, a short description what I did first:
My goal is to build a dapp. As a framework I use React with Typescript.
To start I just created a new React project with npx create-react-app my-app --template typescript
Then I imported Taquito and added a connect button.
When I started the app on localhost I suddenly got a ton of polyfill errors.

After quite some time of breaking walls I figured out that:

  1. Downgrading the react scripts package from v5.0 (which was released four months ago) to v4.0.3 my app runs without any problems! We have the webpack configs in react-scripts and there were some changes, thats why it doesnt work with v5 currently.
    https://www.npmjs.com/package/react-scripts
    But we all can say that this is only a temporary solution.

Taquito needs polyfill and thats an old method that works different in Webpack 5 and Webpack 5 is already around two years old. React is using the higher Webpack versions. While building a library you should not add polyfill and bundle the library. It IS already on the client app. So if you have it and the client app has it, it can easily happen that the versions are different and this will cause conflicts (like in my example).

Considering that there are many different framework(Vue, angular, react..) all have itself different Webpack configs and if you supply that in the library prebundled - you cannot manage and update all different versions so all apps will work. Thats some intense overhead.

Also npm says that people should not bundle that and its kind of bad library design. Please dont get me wrong Taquito is great and I want it to be better!

Now to my feature request to solve this:
So either you:

  1. Taquito does not add polyfill and bundle the library as the process is already done on the client app side. No conflicts can happen.

  2. Or you separate the bundle process from the source process (from your source library files). Because when people use react they can use it without this awful polyfill mistakes.
    Alternatively you could put the bundle process in an own package and distribute the files over npm.

It will make the distribution easier and the accessability will be better too.

Describe alternatives you've considered
An alternative that actually IS NOT an alternative is to do to react eject and insert polyfill BUT PLEASE NOONE SHOULD DO THAT. As when you do that there is no real sense to use react anyway. I just wanted to mention it as I saw it mentioned quite often, so again please dont do that. Its a no go because you lose all react benefits.

@BearCooder
Copy link

Hey @roxaneletourneau whats current ETA here for the addresses issue? I saw in #1544 that you would like to prioritize soon, maybe in version 12.2. And I see version 13.0 is now out :)

@AiMingQi
Copy link

This is also a problem with VueJS.

@roxaneletourneau
Copy link
Collaborator

Hey team! Please add your planning poker estimate with ZenHub @dsawali @hui-an-yang @zainen

@BearCooder
Copy link

Any update about this issue? :)

@roxaneletourneau
Copy link
Collaborator

@BearCooder Hi, sorry for the delay on this issue. We had to reorganize our priority based on the protocol upgrade and its new features. I can not give an ETA at the moment, but we didn't forget about the issue, and it is still high in our backlog.

@roxaneletourneau roxaneletourneau self-assigned this Sep 7, 2022
@BearCooder
Copy link

I see v14 is out, any chance to see it happening in the next milestones? - https://github.com/ecadlabs/taquito/milestones

@claudebarde
Copy link
Contributor

I don't see any reason to change Taquito for that!
You said it yourself, when you downgrade Webpack to version 4, Taquito works right out of the box! So it's not a problem with Taquito. Version 5 of Webpack requires polyfills for different libraries that Taquito uses, it was announced and every other developer has to work with that, you are responsible for updating the configuration of your bundler if you use Webpack or switch to a different bundler that doesn't require any polyfill (never had to use a polyfill with ViteJS!)
Modifying the way Taquito is bundled only to cater to the choices of the Webpack team might require more work down the road for the other developers who use other bundlers. Taquito works so well, that you can use it without any framework or bundler directly in vanilla JavaScript (I tested it)! It's definitely a Webpack problem, not a Taquito problem.

@dvkam
Copy link
Contributor Author

dvkam commented Sep 30, 2022

Well we might have slight different opinions on this topic and thats fine. I look forward to Taquito development and hope this gets at least discussed in the Taquito team and maybe tackled.

@BearCooder
Copy link

I want to add that ultimately Taquito should be easy accessible for every developer. Just stating that everyone is responsible for updating the configuration of your bundler if you use Webpack or switch to a different bundler is not the solution in this particular problem.
Most developers do not want to spend time configuring own bundler but quickstart into the code! For that CRA or other similar tools are a great tool and used by a sheer amount of people. It would be a shame to ignore this big dev group. Sure, some dont mind and start to mess to configure the bundler by itself and will do it but the rest will just leave.
Is everyone responsible for their project configuration? Yes. But regarding maintaining their own configuration or leave it to a tool (e.g. CRA), in a world where frameworks and tools like CRA gain more and more popularity there is no straight Yes or No but different opinions where everyone thinks he is correct. I would even say that the majority of not so experienced devs should not even configure the bundler itself and mess around with it before having a profound knowledge of JS down to the engine (e.g. v8).

For now downgrading to v4 works but ultimately this option will somewhere in the future go away.

You shouldnt bundle your package since it makes tree shaking more complicated for the consumer.

Its great that Taquito works without any framework or bundler in VanillaJS but this is not subject of the topic.
It does not work straight out of the box with React after CRA (unless you downgrade manually react scripts or eject) and I consider this as an entry barrier for new developers that want to start with Tezos.

@claudebarde
Copy link
Contributor

There are many dapps in the ecosystem that are built with React (PlentyDefi comes to mind) and use Taquito, some even use NextJS with React and Taquito. If you choose the easy solution, i.e CRA, you should also be aware of its limitations. You can also use Rollup or ViteJS and React together. The polyfills issue was introduced by Webpack and in my opinion, should be addressed and fixed by React and not by the thousands of NPM libraries out there that use the packages Webpack doesn't polyfill anymore and React makes it difficult to configure.

This is an issue that was discussed among the Taquito team and taken into account, but there are more urgent things to implement right now than catering to the choices of the Webpack and React teams.

This is also a problem that can be easily fixed on your end by ejecting the React app and updating the Webpack configuration, it takes literally 2 minutes. Don't be afraid to eject your React app, nothing bad will happen 🙂

@claudebarde
Copy link
Contributor

Here is a link to a very well-documented comment to get the different steps to install polyfills with Webpack 5 without ejecting the app (it takes less than 5 minutes):
facebook/create-react-app#11756 (comment)

@BearCooder
Copy link

BearCooder commented Oct 5, 2022

Thanks for the link I know this thread and was following it for a long time. It shows exactly our problem across JS ecosystem.
Let me add imo important piece that I should have mentioned in my previous post:

The polyfills issue was introduced by Webpack and in my opinion, should be addressed and fixed by React and not by the thousands of NPM libraries out there that use the packages Webpack doesn't polyfill anymore and React makes it difficult to configure.

Thank god Webpack did this step!
The problem we see here is an assumption if something is using npm so it is technically using node and that is should polyfill everything you can do in node. PLEASE NO. Just because everyone does it it does not mean its correct - a terrible reality in npm land..

Overall the distinction between frontend and backend packages in the node ecosystem is bad (I would even say its horrific). But that does not mean in any world we should polyfill fs, crypto or other into webapps. This is a terrible idea, even considering the idea of polyfilling node builtins, because e.g. you DO NOT call crypto on your client.

Its a historical problem that finally gets eradicated (slow but steady) - CRA used to include polyfills for node js stuff that shouldnt have been running in the browser including crypto. If you imported node crypto modules directly in your component that should not ever have because nodes crypto module IS NOT a browser module but what they did for us was to polyfill those things. Finally React realized that this is probably not a good idea and the team made the right decision with v5 to not longer do this because doing that is just bad.

The vast majority of the things in node were built to run on the server because node is serverside javascript.
It got bastardized into a package manager for client apps but that doesnt mean we should just treat them the same.
Node builtin modules are not in any way built for the browser.

There is actually a new W3C working group who argue about base set of javascript commands and functionality should exist on all platforms ( be it node, deno etc).

Thats why I am a big oponent of the proposed solution also without ejecting. Its simply wrong to do this in my modest opinion.
So be it a priority or not I hope to see the change in the future. 🙂

ps.
I saw also CRACO as solution: Let me elaborate why this is the same bad but even more:

CRACO stands for create react app configuration ovverride. This is one of the obscurest packages ever made. What CRACO does is patch packages parts of create react app to let you do things create react app didnt build in. This is an override layer on top of create react app that assumes an exact version of CRA and then you have to rewrite all of the patches when anything changes in CRACO. Instead of going deeper to solve the problem we build another layer on top of it.
Also CRACO is no longer maintained because.. maintaining that is miserable..

ps.
Nothing more to add 🙂From the thread you linked.
grafik

@BearCooder
Copy link

FYI, the very popular web3.js lib on eth resolved the polyfill/webpack issue in their lateste release.

I did not dive in how, so maybe its still more a kind of "patch" than right solution but what I mean to say is, the peeps over there dont say its the devs responsibility they tried to solve it to improve the dev experience and did it. You asking why Tezos lacks developers? Because of this attitude, sorry to say that. (I have no problem if its taking time though)

@zamrokk
Copy link

zamrokk commented Jan 3, 2023

@BearCooder
Copy link

It's definitely a Webpack problem, not a Taquito problem.

A little bit related :)
https://twitter.com/devongovett/status/1602459843784282113
And this comes not from a webpack dev.

(I still hope to see this happen in the future)

@ac10n
Copy link
Contributor

ac10n commented Jan 25, 2024

@BearCooder @dvkam
I'm working on this issue. Is any of you available to help? I'm on Tezos slack and discord.

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

No branches or pull requests

7 participants