build: change rollup target to ES2018

[amirhhashemi]

Related Issues or Discussions

#48

Summary

Jotai causes a very hard to debug syntax error in browsers that don't support ES2019. I tracked down the issue to the two catch blocks that use Optional catch Binding. Optional catch Binding was introduced in ES2019 and is not supported in Chrome 65 and older. This PR fixes that.

Browser support: https://caniuse.com/mdn-javascript_statements_try_catch_optional_catch_binding

Check List

• yarn run prettier for formatting code and docs

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
jotai	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 26, 2024 5:50am

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[github-actions]

Preview in LiveCodes

Latest commit: 9e1a9a5
Last updated: Feb 26, 2024 5:50am (UTC)

Playground	Link
React demo	https://livecodes.io?x=id/DJKH4DJHL

---

See documentations for usage instructions.

[vincentfretin]

Probably not related to this, I coincidentally just got a sentry error of a user with an iPad with iOS 12.5.7 so probably one of the 10 years old devices like iPad Air, mini 2 or mini 3 that can't update to iOS 13.
I asked gpt-4:
The optional catch binding feature in JavaScript, which allows omitting the parentheses and the error parameter in a catch block when the error is not used, was introduced in ECMAScript 2019 (ES10). Safari started supporting this feature starting from Safari 13.

[dai-shi]

Thanks!
Yeah, I generally avoid omitting () to catch, but forgot in these places.
I wonder if there's an eslint rule to prohibit it.

[amirhhashemi]

I wonder if there's an eslint rule to prohibit it.

Looks like there isn't (and probably won't be) an official rule for that. eslint/eslint#10264

But there are plugins that provide this rule. Like https://eslint-plugin-es.mysticatea.dev/rules/no-optional-catch-binding.html

[dai-shi]

@amirhhashemi
That's a good insight actually.
We should probably change the target.
This seems to work. Can you try it, and change this PR?

diff --git a/rollup.config.js b/rollup.config.js
index 66c8f0e..afd185f 100644
--- a/rollup.config.js
+++ b/rollup.config.js
@@ -33,10 +33,11 @@ function getBabelOptions(targets) {
   }
 }
 
-function getEsbuild(target, env = 'development') {
+function getEsbuild(env = 'development') {
   return esbuild({
     minify: env === 'production',
-    target,
+    target: 'es2018',
+    supported: { 'import-meta': true },
     tsconfig: path.resolve('./tsconfig.json'),
   })
 }
@@ -78,7 +79,7 @@ function createESMConfig(input, output, clientOnly) {
         delimiters: ['\\b', '\\b(?!(\\.|/))'],
         preventAssignment: true,
       }),
-      getEsbuild('node12'),
+      getEsbuild(),
       banner2(() => clientOnly && cscComment),
     ],
   }
@@ -157,7 +158,7 @@ function createSystemConfig(input, output, env, clientOnly) {
         delimiters: ['\\b', '\\b(?!(\\.|/))'],
         preventAssignment: true,
       }),
-      getEsbuild('node12', env),
+      getEsbuild(env),
       banner2(() => clientOnly && cscComment),
     ],
   }

[dai-shi]

#2419 (comment)

[amirhhashemi]

@dai-shi Yeah I think it works. I reverted the old commit and just changed the rollup config.

Before:

$ yarn run build
$ grep -o 'catch{' ./dist/**/*.js
./dist/system/vanilla/utils.production.js:catch{
./dist/system/vanilla/utils.production.js:catch{
./dist/system/vanilla/utils.production.js:catch{

After:

$ yarn run build
$ grep -o 'catch{' ./dist/**/*.js
# There is no 'catch{` in the build output

[dai-shi]

btw you want to grep catch {.

[dai-shi]

Thanks for the change.
(We should probably do the same for Zustand and Valtio.)

[amirhhashemi]

btw you want to grep catch {.

Isn't the build output minified? catch { -> catch{

[dai-shi]

No, for esm and cjs builds, we don't minify.

[amirhhashemi]

I tried catch { and the result is the same:
Before:

$ grep -o 'catch {' ./dist/**/*.js
./dist/system/vanilla/utils.development.js:catch {
./dist/system/vanilla/utils.development.js:catch {

After:

$ grep -o 'catch {' ./dist/**/*.js
# There is no 'catch {` in the build output

[dai-shi]

Interesting. I get this:

$ ag 'catch {' dist|cat        
dist/esm/vanilla/utils.mjs:355:          } catch {
dist/esm/vanilla/utils.mjs:391:          } catch {
dist/system/vanilla/utils.development.js:379:                } catch {
dist/system/vanilla/utils.development.js:415:                } catch {

[amirhhashemi]

Wired. I tried it again and it seems to be working. I even tried a grep that covers more scenarios: grep -o "catch\s*{" ./dist/**/*.{js,ts,mjs,mts,cjs,cts}

Did you run yarn run build before grep?

[dai-shi]

This isn't super important, but just curious.

I run yarn build again and here's the result:

$ grep 'catch {' dist/esm/vanilla/utils.mjs 
          } catch {
          } catch {

[dai-shi]

(We should probably do the same for Zustand and Valtio.)

pmndrs/zustand#2361
pmndrs/valtio#863