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

Simplify build process #117

Open
matheus23 opened this issue Feb 4, 2021 · 7 comments
Open

Simplify build process #117

matheus23 opened this issue Feb 4, 2021 · 7 comments
Assignees
Labels
question Further information is requested

Comments

@matheus23
Copy link
Member

Seeing #116 made me remember I had some questions about the build process in drive:

  • why are we using pnpm?
  • why are we copying files from node_modules into web_modules? (I guess not everything is on npm?)
  • why are we running terser instead of just using esbulid's --minify flag?

Can we simplify the build process?

@matheus23
Copy link
Member Author

Oh and we could use esbuild's --define flag instead of using mustache for providing the constants defined in config/.

@matheus23
Copy link
Member Author

Ok I understand now that during development, we simply don't use any bundling. This is probably some of the reason for some of the things on this list.

Makes me think of snwopack. But in general I'd rather get rid of build depndencies than add more :)

@expede expede added the question Further information is requested label Feb 5, 2021
@icidasset
Copy link
Contributor

icidasset commented Feb 8, 2021

  1. I like pnpm better than npm and yarn. Mostly because the efficient storage method and better errors than yarn. And it's hella fast.
  2. Originally we used snowpack (v1), this was before it had any build tasks. Some dependencies that are on NPM require compilation before they are usable in the browser. But since I avoided webpack and stuff like that, I used services like https://bundle.run/ instead.
  3. We weren't using esbuild until very recently, so we can probably use esbuild here instead yeah.

Good find regarding --define flag, looks useful.

@matheus23
Copy link
Member Author

matheus23 commented Feb 9, 2021

I like pnpm better than npm and yarn. Mostly because the efficient storage method and better errors than yarn. And it's hella fast.

👍

Originally we used snowpack (v1), this was before it had any build tasks. Some dependencies that are on NPM require compilation before they are usable in the browser. But since I avoided webpack and stuff like that, I used services like https://bundle.run/ instead.

I'd be super happy trying out snowpack v3 for drive today! I've had good experiences using it for https://github.com/matheus23/flatmate.
I think it would make the Justfile significantly simpler, too:

  • No need to copy around files from node_modules to web_modules
  • No more similarily structured hot and dev-build tasks. --hot is then only a flag for the snowpack's dev server

We weren't using esbuild until very recently, so we can probably use esbuild here instead yeah.

👍

Good find regarding --define flag, looks useful.

I'm crossing my fingers that it works with snowpack 😅

So, in general, would you be happier with drive being powered by snowpack?

@icidasset
Copy link
Contributor

icidasset commented Feb 10, 2021

Well, we used snowpack previously and moved away from it.
I never use hot reloading personally, so I didn't keep it around for that purpose.
Other than that I don't see much use for it.

The reason we copy those few node modules isn't for Drive itself, it's for the https://github.com/fission-suite/drive/blob/master/src/Static/Html/Reception.html
But yeah I agree, if we'd have extra dependencies, we would need to copy them.

And we'd still need that download task https://github.com/fission-suite/drive/blob/master/Justfile#L73-L76 because those dependencies aren't written in ES6, so you can't use them directly from node_modules with snowpack, even with their commonjs compatibility layer.

@icidasset
Copy link
Contributor

That said, if the HMR is much better for you with snowpack, feel free to add it.

@icidasset
Copy link
Contributor

What I originally loved about snowpack is now https://www.npmjs.com/package/esinstall/
I'll add that soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants