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

[RFC]: ESM (draft) #9665

Closed
1 task
virtuoushub opened this issue Dec 13, 2023 · 1 comment
Closed
1 task

[RFC]: ESM (draft) #9665

virtuoushub opened this issue Dec 13, 2023 · 1 comment

Comments

@virtuoushub
Copy link
Collaborator

virtuoushub commented Dec 13, 2023

Summary

⚠️ - notes from investigating codebase ESM conversion effort, not yet summarized - ⚠️

Resources

Related PRs

Motivation

  1. Reduce bundle size / allow easier tree shaking
  2. Future proof codebase
  3. Make core code easier to work with and less error prone
  4. instill confidence of contributors that refactors and change they make are not introducing regressions

Detailed proposal

  • Dual package to provide Common and ESM to start
  • Use strangler pattern to gradually migrate packages over to ESM
  • Research if adopting a similar approach to ESM conversion for changing test frameworks (vitest, node's test runner is also relevant
  • ...TBD

Are you interested in working on this?

  • I'm interested in working on this

Logs from converting CRWA and adding "type": "module", to package.jsons

In a fresh CRWA (@v6.4.2) via yarn create redwood-app my-esm-redwood-typescript-project --typescript ( in testing, results were the same for clean CRWA javascript project)


after adding "type": "module", to ./web/package.json

and removing "type": "module", to ./api/package.json

...
➜  my-esm-redwood-typescript-project git:(main) ✗ yarn redwood dev
web | failed to load config from /redwood-playground/my-esm-redwood-typescript-project/web/vite.config.ts
web | file:///redwood-playground/my-esm-redwood-typescript-project/web/vite.config.ts.timestamp-1701624570245-c14b8bfb39ab3.mjs:7
web |   plugins: [redwood()]
web |             ^
web |
web | TypeError: redwood is not a function
web |     at file:///redwood-playground/my-esm-redwood-typescript-project/web/vite.config.ts.timestamp-1701624570245-c14b8bfb39ab3.mjs:7:13
web |     at ModuleJob.run (node:internal/modules/esm/module_job:194:25)
web |
web | Node.js v18.17.1
web | yarn cross-env NODE_ENV=development rw-vite-dev  exited with code 1
gen | Generating full TypeScript definitions and GraphQL schemas
gen | Done.
api | Building... Took 301 ms
api | Debugger listening on ws://127.0.0.1:18911/4f543d33-e768-4272-8732-324f7326df89
api | For help, see: https://nodejs.org/en/docs/inspector
api | Starting API Server...
api | Loading server config from /redwood-playground/my-esm-redwood-typescript-project/api/server.config.js
api |
api | Importing Server Functions...
api | 12:29:34 💀
api |  ⚠️ GraphQL server crashed
api |
api |
api | 🚨 Error Info
api |
api | {}
api |
api | 🥞 Error Stack
api |
api | Error: Please use `createValidatorDirective` or `createTransformerDirective` functions to define your directive
api |     at /redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/directives/makeDirectives.js:38:13
api |     at Array.flatMap (<anonymous>)
api |     at makeDirectivesForPlugin (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/directives/makeDirectives.js:25:82)
api |     at createGraphQLYoga (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/createGraphQLYoga.js:54:75)
api |     at createGraphQLHandler (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/functions/graphql.js:51:48)
api |     at Object.<anonymous> (/redwood-playground/my-esm-redwood-typescript-project/api/src/functions/graphql.ts:10:24)
api |     at Module._compile (node:internal/modules/cjs/loader:1256:14)
api |     at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
api |     at Module.load (node:internal/modules/cjs/loader:1119:32)
api |     at Module._load (node:internal/modules/cjs/loader:960:12)
api |
...

after adding "type": "module", to ./api/package.json

...
➜  my-esm-redwood-typescript-project git:(main) ✗ yarn redwood dev
web |   ➜  Local:   http://localhost:8910/
web |   ➜  Network: http://192.168.0.71:8910/
web | 12:22:24 PM [vite] http proxy error at /graphql:
web | ⌛ API Server launching, please refresh your page...
gen | Generating full TypeScript definitions and GraphQL schemas
gen | Done.
api | Building... Took 397 ms
api | Debugger listening on ws://127.0.0.1:18911/0cc2f2dd-8507-40a7-9f1c-41d749c50318
api | For help, see: https://nodejs.org/en/docs/inspector
api | Starting API Server...
api | Loading server config from /redwood-playground/my-esm-redwood-typescript-project/api/server.config.js
api |
api | /redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/fastify.js:41
api |       ...require(serverConfigPath)
api |          ^
api |
api | Error [ERR_REQUIRE_ESM]: require() of ES Module /redwood-playground/my-esm-redwood-typescript-project/api/server.config.js from /redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/fastify.js not supported.
api | server.config.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
api | Instead rename server.config.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /redwood-playground/my-esm-redwood-typescript-project/api/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).
api |
api |     at loadFastifyConfig (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/fastify.js:41:10)
api |     at createFastifyInstance (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/fastify.js:50:7)
api |     at apiServerHandler (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/cliHandlers.js:94:38)
api |     at Object.<anonymous> (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/api-server/dist/index.js:37:39) {
api |   code: 'ERR_REQUIRE_ESM'
api | }
api |
api | Node.js v18.17.1

...

after adding just "type": "module", to ./package.json

...
➜  my-esm-redwood-typescript-project git:(main) yarn redwood dev
web |   ➜  Local:   http://localhost:8910/
web |   ➜  Network: http://192.168.0.71:8910/
web | 12:15:39 PM [vite] http proxy error at /graphql:
web | ⌛ API Server launching, please refresh your page...
gen | Generating full TypeScript definitions and GraphQL schemas
gen | Done.
api | Building... Took 336 ms
api | Debugger listening on ws://127.0.0.1:18911/f3ced000-f714-465c-a326-7c3436cefbf7
api | For help, see: https://nodejs.org/en/docs/inspector
api | Starting API Server...
api | Loading server config from /redwood-playground/my-esm-redwood-typescript-project/api/server.config.js
api |
api | Importing Server Functions...
api | 12:15:42 💀
api |  ⚠️ GraphQL server crashed
api |
api |
api | 🚨 Error Info
api |
api | {}
api |
api | 🥞 Error Stack
api |
api | Error: Please use `createValidatorDirective` or `createTransformerDirective` functions to define your directive
api |     at /redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/directives/makeDirectives.js:38:13
api |     at Array.flatMap (<anonymous>)
api |     at makeDirectivesForPlugin (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/directives/makeDirectives.js:25:82)
api |     at createGraphQLYoga (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/createGraphQLYoga.js:54:75)
api |     at createGraphQLHandler (/redwood-playground/my-esm-redwood-typescript-project/node_modules/@redwoodjs/graphql-server/dist/functions/graphql.js:51:48)
api |     at Object.<anonymous> (/redwood-playground/my-esm-redwood-typescript-project/api/src/functions/graphql.ts:10:24)
api |     at Module._compile (node:internal/modules/cjs/loader:1256:14)
api |     at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
api |     at Module.load (node:internal/modules/cjs/loader:1119:32)
api |     at Module._load (node:internal/modules/cjs/loader:960:12)
api |

...

This logic might need updating? if (!directive.type) { -> if (!directive.type && !directive.default.type) { "works", but does not seem like a implementation fix we want.

https://github.com/redwoodjs/redwood/blame/main/packages/graphql-server/src/directives/makeDirectives.ts#L36-L43

related to ajv-validator/ajv#1381


@virtuoushub
Copy link
Collaborator Author

seems like this isn't needed anymore #9667 (comment)

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

1 participant