Test bundler module res #124

anncatton · 2023-10-05T12:15:11Z

Converting all apps and packages to use the ESM format, EXCLUDING consent-ui. This is the general direction JS is moving and we have already run into issues with newer versions of external libraries, i.e. url-join and nanoid.

Change summary:

package.json files:

type: module
enforce node version 18 or higher (with .npmrc engine-strict=true setting)
set apps' "exports" to null as these shouldn't need to be imported anywhere
switch "main" to "exports" field to match esm recommendation
for all apps, start script will use ts-node to run from the src folder to avoid an issue encountered with the prisma generated client folder, which was not being added to the dist folder on build, as per @justincorrigible 's suggestion
updates all "workspace:^" references to use the ^ instead of * syntax, for consistency. This is how it appears when installing a local package with pnpm add <package>

tsconfig.base.json:

default module and moduleResolution settings will be NodeNext as recommended here, target becomes ESNext

extended tsconfig.json files:

set tsnode.esm to true, cannot run without this
set tsnode.logError to true to prevent apps running start script with ts-node from crashing if any type errors are encountered

consent-ui:

overriding the base tsconfig with the Bundler option recommended here, to continue taking advantage of nextjs magic.
not setting this app as type: module because this broke the postcss/tailwind styles
removed the @ alias for absolute imports, but will continue using this kind of import here

General:

removes absolute paths as these do not work with esm modules.
adds a .js file extensions to all imports, the path cannot be resolved properly otherwise
adds version import from package.json in all apps. assert { type: 'json' } required on the import to align with changes in Node 17, as mentioned here
sets all package versions to 0.1.0

Packages:

common renamed to types
restructured types folders
modified types package.json to allow submodule imports, and updates all import statements

Documentation will be added for all of this in #128

…atform into test-bundler-module-res

anncatton · 2023-10-05T19:15:23Z

apps/consent-ui/src/components/Link/LocalizedLink.tsx

@@ -35,7 +36,7 @@ const LocalizedLink = ({
 	...rest
 }: LocalizedLinkProps) => {
 	let locale = linkLang;


Haven't found a nice way to import these routes, so far only a json file has worked with the nextjs.config rewrites. Adding a TODO to look at a way to get rid of the as here

justincorrigible · 2023-10-05T19:30:32Z

apps/consent-api/src/routers/status.ts


-import { version } from '@/../../package.json';
-import logger from '@/logger';
+import packageJson from '../../package.json' assert { type: 'json' };


these absolute import paths can be troubleshot. I have a fix for them (to be applied in the next PR)

@joneubank pointed out that absolute paths, while looking nice and tidy, can actually add confusion, and the esm standard is for relative paths anyway

they can also cleanup in some cases. ideally we could use both.

you're right, relative is the ESM standard, but they make PR reviewing (as you don't have a full IDE in github), and they also make app onboarding so much harder (chasing the "dot dot rabbit" down its hole, vs "ah, this comes from the global utils" at first glance).

besides, just like having types in a js file (which is not js spec either), absolute paths are something we can handle through TS transpilation

justincorrigible · 2023-10-05T19:35:43Z

apps/consent-das/prisma/seed/seed-data.ts

@@ -1,4 +1,4 @@
-import { Prisma, ConsentCategory } from '../../src/generated/client';
+import { Prisma, ConsentCategory } from '../../src/generated/client/index.js';


oof. now this one "requirement" will sting for a lot of people 🤷

justincorrigible · 2023-10-05T19:37:19Z

apps/consent-ui/src/app/[lang]/consent/step-1/page.tsx


-import { ValidLanguage } from '@/i18n';
-import ConsentForm from '@/components/ConsentForm';
+import { ValidLanguage } from 'src/i18n';


does this absolute path work because of nextjs?

yes appears it is nextjs + the bundler setting magic

justincorrigible · 2023-10-06T12:37:44Z

packages/types/package.json

-		"node": ">=18"
-	},
+    "node": ">=18"
+  },


curious about this change. is this project using tabs or spaces?

edit: checked, it's spaces. I was under the impression we were going to use tabs in all new projects 🤔
did we change direction?

I think the linter settings (which apply things like tabs instead of spaces on fix-on-save) haven't been configured for all files in the repo (like package.json files), and when I was initially setting up the repo I was still using spaces so this may be a holdover from back then

In other words... my bad 🙈 This one's totally on me haha

absolutely terrible. I'm heartbroken!!
how dare you!? 😂

/s

apps/data-mapper/tsconfig.json

packages/types/src/entities/ConsentCategory.ts

packages/types/src/entities/User.ts

packages/types/src/entities/index.ts

Co-authored-by: Anders Richardsson <2107110+justincorrigible@users.noreply.github.com>

mistryrn

Looks good! Tested locally and confirmed things are still building, running in dev, and now actually running with the start command too 😅 Thanks so much for going down the rabbit holes required to get this all in! 🙌 🙇

Other than the little README update we talked about on Slack, this looks good to go 🚀

…atform into test-bundler-module-res

mistryrn · 2023-10-06T15:57:51Z

README.md

@@ -41,12 +41,14 @@ The directory structure is as follows:
 │       └── src
 ├── docker-scripts/                 ← Docker Init Scripts
 └── packages/
-    ├── common/                     ← Shared Validation & Types
-    │   └── src
+    ├── types/                      ← Shared Validation & Types/


nitpick: Looks like we've got some rogue trailing slashes here: & Types/, Logger/, APIs/ -- also do you mind putting these in alphabetical order like they appear in the directory?

mistryrn

🚀

mistryrn

🚀 (for reals this time)

anncatton and others added 12 commits October 4, 2023 16:43

update data-mapper to use esm

cb0b07f

rename common to types, make all packages esm modules

c375c2c

update consent-ui imports, module res to bundler

2ec034b

tsconfig, package.json, import statement updates for esm

11dddf7

tsconfig, package.json, import paths updates for using esm in phi-das

14146c1

tsconfig, package.json, import path updates for esm in pi-das

1e31fb8

tsconfig, package.json, import path updates for esm in consent-das

45fe076

tsconfig, package.json, import path updates for esm in consent-api

6d21281

set all packages, apps to v0.1.0 as a base, app exports field to null

e3e185c

Merge branch 'develop' into test-bundler-module-res

fa539eb

update workspace package references to use consistent naming

3d3b2de

Merge branch 'test-bundler-module-res' of github.com:OHCRN/consent-pl…

d14e7b0

…atform into test-bundler-module-res

anncatton commented Oct 5, 2023

View reviewed changes

anncatton marked this pull request as ready for review October 5, 2023 19:20

anncatton requested review from mistryrn, justincorrigible and joneubank October 5, 2023 19:20

add todo

0f1a41e

anncatton mentioned this pull request Oct 5, 2023

Wiki documentation for ESM migration #128

Open

justincorrigible reviewed Oct 5, 2023

View reviewed changes

anncatton mentioned this pull request Oct 5, 2023

🔧 🎨 allow submodule imports from common package #102

Closed

add eslint dep and config to types package

7ba1595

justincorrigible reviewed Oct 6, 2023

View reviewed changes

apps/data-mapper/tsconfig.json Outdated Show resolved Hide resolved

justincorrigible reviewed Oct 6, 2023

View reviewed changes

packages/types/src/entities/ConsentCategory.ts Outdated Show resolved Hide resolved

justincorrigible reviewed Oct 6, 2023

View reviewed changes

packages/types/src/entities/User.ts Outdated Show resolved Hide resolved

justincorrigible reviewed Oct 6, 2023

View reviewed changes

packages/types/src/entities/index.ts Outdated Show resolved Hide resolved

justincorrigible previously approved these changes Oct 6, 2023

View reviewed changes

anncatton and others added 2 commits October 6, 2023 09:07

formatting

5eabbcd

Update apps/data-mapper/tsconfig.json

d9bc409

Co-authored-by: Anders Richardsson <2107110+justincorrigible@users.noreply.github.com>

anncatton dismissed justincorrigible’s stale review via d9bc409 October 6, 2023 14:26

justincorrigible previously approved these changes Oct 6, 2023

View reviewed changes

mistryrn previously approved these changes Oct 6, 2023

View reviewed changes

anncatton added 2 commits October 6, 2023 11:29

update README with types and error-handler packages

6f2d01a

Merge branch 'test-bundler-module-res' of github.com:OHCRN/consent-pl…

1819a8b

…atform into test-bundler-module-res

anncatton dismissed stale reviews from mistryrn and justincorrigible via 1819a8b October 6, 2023 15:29

mistryrn reviewed Oct 6, 2023

View reviewed changes

set apps noEmit to true, add missing package version to data-mapper

b08d54e

mistryrn previously approved these changes Oct 6, 2023

View reviewed changes

data-mapper start script to use ts-node

b62d927

anncatton dismissed mistryrn’s stale review via b62d927 October 6, 2023 16:35

add logError option to data-mapper ts-node config

eab2dbd

mistryrn approved these changes Oct 6, 2023

View reviewed changes

anncatton merged commit 4abd47e into develop Oct 6, 2023
3 checks passed

anncatton deleted the test-bundler-module-res branch October 6, 2023 16:57

anncatton restored the test-bundler-module-res branch October 6, 2023 16:57

anncatton deleted the test-bundler-module-res branch October 6, 2023 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test bundler module res #124

Test bundler module res #124

anncatton commented Oct 5, 2023 •

edited

Loading

anncatton Oct 5, 2023

justincorrigible Oct 5, 2023 •

edited

Loading

anncatton Oct 5, 2023

joneubank Oct 5, 2023

justincorrigible Oct 5, 2023 •

edited

Loading

justincorrigible Oct 5, 2023

anncatton Oct 5, 2023

justincorrigible Oct 5, 2023

anncatton Oct 5, 2023

justincorrigible Oct 6, 2023 •

edited

Loading

mistryrn Oct 6, 2023

justincorrigible Oct 6, 2023

mistryrn left a comment

mistryrn Oct 6, 2023

mistryrn left a comment

mistryrn left a comment

		@@ -1,4 +1,4 @@
		import { Prisma, ConsentCategory } from '../../src/generated/client';
		import { Prisma, ConsentCategory } from '../../src/generated/client/index.js';

Test bundler module res #124

Test bundler module res #124

Conversation

anncatton commented Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

justincorrigible Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justincorrigible Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justincorrigible Oct 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistryrn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistryrn left a comment

Choose a reason for hiding this comment

mistryrn left a comment

Choose a reason for hiding this comment

anncatton commented Oct 5, 2023 •

edited

Loading

justincorrigible Oct 5, 2023 •

edited

Loading

justincorrigible Oct 5, 2023 •

edited

Loading

justincorrigible Oct 6, 2023 •

edited

Loading