search

triggerdotdev / jsonhero-web

JSON Hero is an open-source, beautiful JSON explorer for the web that lets you browse, search and navigate your JSON files at speed. 🚀. Built with 💜 by the Trigger.dev team.
https://jsonhero.io
Apache License 2.0
9.01k stars 482 forks source link

upgrade to version remix version 1.16.1 #161

Closed scshiv29-dev closed 12 months ago

scshiv29-dev commented 1 year ago

Upgraded the remix version along with @remix-run/react @remix-run/node @remix-run/serve and added @remix-run/cloudfare because it was the solution for "fs/promise" error i was getting with @remix-run/node .

/claim #15

lmk if there are any other changes in this code base i need to make

ericallam commented 1 year ago

After pulling this PR and running npm i I got this error:

jsonhero.io $ npm i

> postinstall
> remix setup cloudflare-workers

ENOENT: no such file or directory, scandir '/Users/eric/code/APIHero/jsonhero/jsonhero.io/node_modules/@remix-run/react/magicExports'
npm ERR! code 1
npm ERR! path /Users/eric/code/APIHero/jsonhero/jsonhero.io
npm ERR! command failed
npm ERR! command sh -c -- remix setup cloudflare-workers

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/eric/.npm/_logs/2023-05-24T20_37_07_915Z-debug-0.log

Could you investigate whether the remix setup cloudflare-workers postinstall script is still necessary after the upgrade?

scshiv29-dev commented 1 year ago

@ericallam sure will take a look

scshiv29-dev commented 1 year ago

@ericallam so I checked and it is still necessary for us to use remix setup cloudflare-workers

the above error is because they removed magicExports completly. referred to this discussion here I also upgraded some packages that I missed in my first commit and ran npm i and then it is working

image

also ran with postinstall script remix setup cloudflare and it is also working so i kept the new version. image

could you pull this pr and check if this is working ? thanks

ericallam commented 1 year ago

npm i now works (nice one!) but I'm getting issues with tailwind when running the app locally with npm start. I think we'll need to upgrade the tailwind stuff to use the new built-in Remix support for tailwind:

https://remix.run/docs/en/1.16.0/guides/styling#tailwind-css

scshiv29-dev commented 1 year ago

@ericallam even i was getting the tailwind error when i tried to deploy it , let me take a look on how to fix it

scshiv29-dev commented 1 year ago

@ericallam so i added eyeicon to external and that seems to fix the issue . Can you take a look at the pr ? also this conflict is due to the latest merge of #160 .

scshiv29-dev commented 1 year ago

@ericallam did you get a chance to review the PR ?

matt-aitken commented 1 year ago

Eric is away so I'll take this over.

I did a fresh checkout of this branch and followed the steps in the DEVELOPMENT.md. I get this error when running npm start:

[0] [2] 💿 Building...
[1] [mf:err] /Users/matt/Desktop/jsonhero/dist/worker.js:5855
[1]         LANG: navigator.language + ".UTF-8",
[1]               ^
[1] 
[1] ReferenceError: navigator is not defined
[1]     at node_modules/@jspm/core/nodelibs/browser/process.js (/Users/matt/Desktop/jsonhero/node_modules/@jspm/core/nodelibs/browser/process.js:70:9)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/esbuild-plugin-polyfill-node/polyfills/process.js (/Users/matt/Desktop/jsonhero/node_modules/esbuild-plugin-polyfill-node/polyfills/process.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/@jspm/core/nodelibs/browser/buffer.js (/Users/matt/Desktop/jsonhero/node_modules/@jspm/core/nodelibs/browser/buffer.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/esbuild-plugin-polyfill-node/polyfills/buffer.js (/Users/matt/Desktop/jsonhero/node_modules/esbuild-plugin-polyfill-node/polyfills/buffer.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at /Users/matt/Desktop/jsonhero/build/<stdin>:1:1
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:95025:3
[1] [mf:err] Unhandled Promise Rejection: /Users/matt/Desktop/jsonhero/dist/worker.js:5855
[1]         LANG: navigator.language + ".UTF-8",
[1]               ^
[1] 
[1] ReferenceError: navigator is not defined
[1]     at node_modules/@jspm/core/nodelibs/browser/process.js (/Users/matt/Desktop/jsonhero/node_modules/@jspm/core/nodelibs/browser/process.js:70:9)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/esbuild-plugin-polyfill-node/polyfills/process.js (/Users/matt/Desktop/jsonhero/node_modules/esbuild-plugin-polyfill-node/polyfills/process.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/@jspm/core/nodelibs/browser/buffer.js (/Users/matt/Desktop/jsonhero/node_modules/@jspm/core/nodelibs/browser/buffer.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at node_modules/esbuild-plugin-polyfill-node/polyfills/buffer.js (/Users/matt/Desktop/jsonhero/node_modules/esbuild-plugin-polyfill-node/polyfills/buffer.js:1:1)
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:5610:59
[1]     at /Users/matt/Desktop/jsonhero/build/<stdin>:1:1
[1]     at /Users/matt/Desktop/jsonhero/dist/worker.js:95025:3

Try a fresh checkout, or delete all of the build folders. Let me know if you can't reproduce after doing that.

scshiv29-dev commented 1 year ago

@matt-aitken if i delete build folder then the worker.js file throws error as during the build it is required

I am still able to build the project on my personal docker and also on vercel on the same branch issue-15 and now I am more confused image somehow getting this error when i delete the build folder and run npm start

vercel Logs 

[14:35:39.793] Running build in San Francisco, USA (West) – sfo1
[14:35:39.824] Cloning github.com/scshiv29-dev/jsonhero-web (Branch: issue-15, Commit: d663328)
[14:35:40.045] Previous build caches not available
[14:35:40.985] Cloning completed: 1.162s
[14:35:41.115] Running "vercel build"
[14:35:41.580] Vercel CLI 29.3.6
[14:35:42.136] Warning: Detected "engines": { "node": ">=14" } in your `package.json` that will automatically upgrade when a new major Node.js Version is released. Learn More: http://vercel.link/node-version
[14:35:42.151] Installing dependencies...
[14:35:43.151] npm WARN EBADENGINE Unsupported engine {
[14:35:43.151] npm WARN EBADENGINE   package: '@jsonhero/fuzzy-json-search@0.2.1',
[14:35:43.151] npm WARN EBADENGINE   required: { node: '16' },
[14:35:43.151] npm WARN EBADENGINE   current: { node: 'v18.15.0', npm: '9.5.0' }
[14:35:43.151] npm WARN EBADENGINE }
[14:35:43.151] npm WARN EBADENGINE Unsupported engine {
[14:35:43.151] npm WARN EBADENGINE   package: '@jsonhero/json-infer-types@1.2.11',
[14:35:43.151] npm WARN EBADENGINE   required: { node: '16' },
[14:35:43.152] npm WARN EBADENGINE   current: { node: 'v18.15.0', npm: '9.5.0' }
[14:35:43.152] npm WARN EBADENGINE }
[14:35:43.153] npm WARN EBADENGINE Unsupported engine {
[14:35:43.153] npm WARN EBADENGINE   package: '@jsonhero/json-schema-fns@0.0.1',
[14:35:43.153] npm WARN EBADENGINE   required: { node: '16' },
[14:35:43.153] npm WARN EBADENGINE   current: { node: 'v18.15.0', npm: '9.5.0' }
[14:35:43.153] npm WARN EBADENGINE }
[14:35:43.153] npm WARN EBADENGINE Unsupported engine {
[14:35:43.153] npm WARN EBADENGINE   package: '@jsonhero/schema-infer@0.1.4',
[14:35:43.153] npm WARN EBADENGINE   required: { node: '16' },
[14:35:43.153] npm WARN EBADENGINE   current: { node: 'v18.15.0', npm: '9.5.0' }
[14:35:43.154] npm WARN EBADENGINE }
[14:35:46.320] npm WARN deprecated rollup-plugin-inject@3.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-inject.
[14:36:01.160] 
[14:36:01.160] > postinstall
[14:36:01.160] > remix setup cloudflare
[14:36:01.160] 
[14:36:02.074] Successfully setup Remix for cloudflare.
[14:36:02.098] 
[14:36:02.098] added 1468 packages in 20s
[14:36:02.098] 
[14:36:02.098] 209 packages are looking for funding
[14:36:02.098]   run `npm fund` for details
[14:36:02.391] Detected `package-lock.json` generated by npm 7+...
[14:36:02.392] Running "npm run build"
[14:36:02.635] 
[14:36:02.635] > build
[14:36:02.636] > npm run build:css && npm run build:search && remix build
[14:36:02.636] 
[14:36:02.867] 
[14:36:02.867] > build:css
[14:36:02.867] > tailwindcss -i ./styles/tailwind.css -o ./app/tailwind.css --minify
[14:36:02.867] 
[14:36:03.101] 
[14:36:03.101] warn - The `purge`/`content` options have changed in Tailwind CSS v3.0.
[14:36:03.102] warn - Update your configuration file to eliminate this warning.
[14:36:03.102] warn - https://tailwindcss.com/docs/upgrade-guide#configure-content-sources
[14:36:03.313] Browserslist: caniuse-lite is outdated. Please run:
[14:36:03.314]   npx browserslist@latest --update-db
[14:36:03.314]   Why you should do it regularly: https://github.com/browserslist/browserslist#browsers-data-updating
[14:36:03.406] 
[14:36:03.406] warn - The `@variants` directive has been deprecated in Tailwind CSS v3.0.
[14:36:03.406] warn - Use `@layer utilities` or `@layer components` instead.
[14:36:03.406] warn - https://tailwindcss.com/docs/upgrade-guide#replace-variants-with-layer
[14:36:03.567] 
[14:36:03.568] Done in 202ms.
[14:36:03.812] 
[14:36:03.812] > build:search
[14:36:03.813] > esbuild ./app/entry.worker.ts --outfile=./public/entry.worker.js --bundle --format=esm --define:process.env.NODE_ENV='"production"'
[14:36:03.813] 
[14:36:03.857] 
[14:36:03.857]   public/entry.worker.js  650.0kb
[14:36:03.857] 
[14:36:03.857] ⚡ Done in 38ms
[14:36:04.662] Building Remix app in production mode...
[14:36:04.669] ⚠️ REMIX FUTURE CHANGE: The behaviors of `CatchBoundary` and `ErrorBoundary` are changing in v2. You can prepare for this change at your convenience with the `v2_errorBoundary` future flag. For instructions on making this change see https://remix.run/docs/en/v1.15.0/pages/v2#catchboundary-and-errorboundary
[14:36:04.670] ⚠️ REMIX FUTURE CHANGE: APIs that provide `formMethod` will be changing in v2. All values will be uppercase (GET, POST, etc.) instead of lowercase (get, post, etc.) You can prepare for this change at your convenience with the `v2_normalizeFormMethod` future flag. For instructions on making this change see https://remix.run/docs/en/v1.15.0/pages/v2#formMethod
[14:36:04.670] ⚠️ REMIX FUTURE CHANGE: The route `meta` export signature is changing in v2. You can prepare for this change at your convenience with the `v2_meta` future flag. For instructions on making this change see https://remix.run/docs/en/v1.15.0/pages/v2#meta
[14:36:04.670] ⚠️ REMIX FUTURE CHANGE: The route file convention is changing in v2. You can prepare for this change at your convenience with the `v2_routeConvention` future flag. For instructions on making this change see https://remix.run/docs/en/v1.15.0/pages/v2#file-system-route-convention
[14:36:05.151] lodash-es is possibly an ESM only package and should be bundled with "serverDependenciesToBundle" in remix.config.js.
[14:36:05.292] @js-temporal/polyfill is possibly an ESM only package and should be bundled with "serverDependenciesToBundle" in remix.config.js.
[14:36:07.022] Built in 2.3s
[14:36:10.725] Build Completed in /vercel/output [29s]
[14:36:10.906] Deploying outputs...
[14:36:15.772] Deployment completed
[14:36:22.775] Uploading build cache [42.78 MB]...
[14:36:23.807] Build cache uploaded: 1.032s

will figure out the issue by doing a new checkout of main branch and making appropriate changes step by step with new version and will probably stumble upon the change that is giving this error. let me keep working on this thanks

scshiv29-dev commented 1 year ago

Hey @matt-aitken @ericallam I redid everything spent 12 straight hour on this tried different approaches and I still ended up getting the same error and it is because of esbuild-plugin-polyfill-node issue in remix new version I would suggest waiting for them to fix it and them change the version for production. I have made all the necessary changes required for new versions .

matt-aitken commented 1 year ago

@scshiv29-dev I haven't forgotten about this. I will discuss this with @ericallam when he's back on Monday morning.

Sorry that this has been such a nightmare and out of your control. The most likely outcome is we award you the bounty and leave this PR here until Remix fix the issue.

scshiv29-dev commented 1 year ago

@matt-aitken You don't have to be sorry .Once the bug gets fixed from their end I will update my pr accordingly . Thanks for updating the timeline though .

scshiv29-dev commented 1 year ago

hey @matt-aitken any updates regarding this ?

ericallam commented 12 months ago

It looks like the issue was fixed in Remix 1.17.1: https://github.com/remix-run/remix/releases/tag/remix%401.17.1

Can you upgrade and test that it fixes the issue? Once that's done we can merge this 👍

scshiv29-dev commented 12 months ago

@ericallam created a new pr #167