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

Transpile JS or Use less modern syntax #3

Closed
meodai opened this issue Sep 7, 2021 · 16 comments
Closed

Transpile JS or Use less modern syntax #3

meodai opened this issue Sep 7, 2021 · 16 comments

Comments

@meodai
Copy link
Contributor

meodai commented Sep 7, 2021

To make sure it works in safari

@nico-martin
Copy link
Contributor

I can have a look at this if you want to.

@meodai
Copy link
Contributor Author

meodai commented Sep 7, 2021

@nico-martin uh yes please!

@nico-martin
Copy link
Contributor

I implemented it on a feature branch: https://github.com/nico-martin/color-description/tree/babel-transpile
But it looks a bit hacky. The key is that I needed to separate JS and HTML so I can transpile the JS in the build.js.
This also means that lib.js (formerly index.js) can't export ColorDescription, but it exposes it as a window-var. Otherwise we would need a bundler that bundles lib.js and index.js into one file (no import/export in the browser).

Oh and since there are now two "source" files, I added a src folder. And I added prettier. I just think its easier if everyone uses the same code formatting. But I'm happy to remove it if you disagree.

What do you think? too much build logic?

@meodai meodai closed this as completed Sep 7, 2021
@meodai meodai reopened this Sep 7, 2021
@meodai
Copy link
Contributor Author

meodai commented Sep 7, 2021

@nico-martin ups, look like it broke the tests :D I also think the package.json needs to be adapted, so browser and note will require the adequate version.

@wooorm
Copy link
Member

wooorm commented Sep 8, 2021

Btw, esbuild is a great bundler. It’s extremely fast!
Here’s how I use it to produce module/nomodule builds: https://github.com/wooorm/write-music/blob/ed0361cfc3e6b143f24b232430c71324f68676be/package.json#L34.
Used like so: https://github.com/wooorm/write-music/blob/ed0361cfc3e6b143f24b232430c71324f68676be/src/index.html#L18-L19, it means modern browsers will pick the modern smaller build, and older browsers the bigger build

@nico-martin
Copy link
Contributor

@meodai Who needs test if your code works😊. No no, I will look into it.

@nico-martin
Copy link
Contributor

@wooorm yeah, I was thinking about a bundling process as well. But I did not know if maybe @meodai wanted to keep the processing as "slim" as possible..

@meodai
Copy link
Contributor Author

meodai commented Sep 8, 2021

@wooorm sounds great! Do you want to have a look @nico-martin. I don't have any feelings towards the "slimness" of the build process ;) as long as npm run builds builds it and import {bla} from bla imports the right thing in the right context.

@nico-martin
Copy link
Contributor

Sure, I will add esbuild for a build process. But this will require some refactoring😊

@meodai
Copy link
Contributor Author

meodai commented Sep 8, 2021

@nico-martin what I also miss now: before I could work on the index file, an instantly see the result in the browser, what made it super easy to work on the class, now I have to build it every time. Not really a viable option imho.

@wooorm
Copy link
Member

wooorm commented Sep 8, 2021

@meodai
Copy link
Contributor Author

meodai commented Sep 8, 2021

@wooorm It works, I will push soon, thanks so much. I am now looking up how I can set the main file in the package.json so the right one is loaded by the right env

@meodai
Copy link
Contributor Author

meodai commented Sep 8, 2021

It now works. But I have a question: The --bundle argument now always bundles chroma-js in. That's fine for a browser package. But in when using import in an environment that supports it. it should not do that. Thats why I had theis node.tpl before. Any idea how to address that?

@meodai
Copy link
Contributor Author

meodai commented Sep 8, 2021

@wooorm ok I guess all I needed is to not use --bundle ;D https://github.com/words/color-description/blob/main/package.json#L9

@wooorm
Copy link
Member

wooorm commented Sep 8, 2021

I’m personally on board of the ESM train. All packages I maintain are ESM only. I recently analyzed all of npm, which you might find interesting: https://twitter.com/wooorm/status/1431289978961764357.
More info on ESM and using it can be found here: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c.

CJS users can use a dynamic import(), but otherwise don’t have access to the code.

People writing code for browsers-only often already have some experience with or existing setup for loading packages from npm, and alternatively all evergreen browsers support ESM as well, so they could use CDNs such as skypack, esm.sh, and the like.

That means I don’t deal with bundling or compiling the code in my packages at all. And for little websites I use esbuild to make module/nomodule bundles.

One example is double-metaphone here: https://github.com/words/double-metaphone.

Glad to hear you got a setup working btw!

@meodai
Copy link
Contributor Author

meodai commented Sep 12, 2021

looks like i managed to do something :D thanks @wooorm !

@meodai meodai closed this as completed Sep 13, 2021
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

3 participants