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.

Sign up for GitHub

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

Jump to bottom

build(client-tree): exports with ESM as primary #19141

Merged
merged 22 commits into from
Feb 10, 2024
Merged

build(client-tree): exports with ESM as primary #19141

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e07e8a8
build: tsc-multi for cjs renames .d.ts
jason-ha Dec 22, 2023
c1e6092
build(tree): settings for ESM as primary
jason-ha Dec 22, 2023
7c75513
build(client-tree): CJS via tsc
jason-ha Dec 22, 2023
ee1576a
test(client-tree): test commonjs
jason-ha Jan 8, 2024
7c064e0
build(client-tree): exclude fluid-build-tasks-tsc policy
jason-ha Jan 8, 2024
c0f68c0
build(client-tree): set package exports
jason-ha Jan 8, 2024
b6d1d5e
build(client-tree): cleanup build dependencies
jason-ha Jan 8, 2024
a9bccf8
build(client-tree): full ESM build
jason-ha Jan 8, 2024
8978bdd
build(client-tree): add api task
jason-ha Jan 8, 2024
4d252b7
build(client-tree): fix api report/rollup
jason-ha Jan 8, 2024
5655ba2
build(client-tree): improve api report/rollup
jason-ha Jan 8, 2024
eef3130
revert(build-common): don't update tsc-multi.cjs.json
jason-ha Jan 8, 2024
4ff9896
merge: main with tree-exports-with-esm-as-primary
jason-ha Jan 11, 2024
000d231
merge: main with tree-exports-with-esm-as-primary
jason-ha Jan 16, 2024
31c512b
fix(tree): fix CJS package.json injection
jason-ha Jan 16, 2024
0bcd137
build(client-tree): CJS explicit moduleResolution
jason-ha Jan 17, 2024
695cb0c
merge: main with tree-exports-with-esm-as-primary
jason-ha Feb 3, 2024
40bdc4b
build(client-tree): restore build:genver
jason-ha Feb 7, 2024
c8b2779
merge: main with tree-exports-with-esm-as-primary
jason-ha Feb 7, 2024
7c5a038
Merge branch 'main' into tree-exports-with-esm-as-primary
jason-ha Feb 9, 2024
3bc2c4c
Merge branch 'main' into tree-exports-with-esm-as-primary
jason-ha Feb 9, 2024
ccd598b
build(client): fix internal tree imports
jason-ha Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/build/build-common/tsc-multi.cjs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"targets": [{ "extname": ".cjs", "module": "node16" }],
"targets": [{ "extname": ".cjs", "dExtName": ".d.cts", "module": "node16", "outDir": "./dist" }],
jason-ha marked this conversation as resolved.
Show resolved Hide resolved
"projects": ["tsconfig.json"]
}
4 changes: 4 additions & 0 deletions fluidBuild.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ module.exports = {
"^packages/utils/.*/package.json",
"^packages/loader/container-loader/package.json",
],
"fluid-build-tasks-tsc": [
"packages/dds/tree/package.json", // Builds CommonJS with custom tsconfig
],
"html-copyright-file-header": [
// Tests generate HTML "snapshot" artifacts
"tools/api-markdown-documenter/src/test/snapshots/.*",
Expand Down Expand Up @@ -231,6 +234,7 @@ module.exports = {
"^tools/getkeys",
],
"npm-package-json-esm": [
"packages/dds/tree/package.json", // Policy is incorrect about "module" in package.json
// These are ESM-only packages and use tsc to build the ESM output. The policy handler doesn't understand this
// case.
"packages/dds/migration-shim/package.json",
Expand Down
0 packages/dds/tree/.dependency-cruiser.js → packages/dds/tree/.dependency-cruiser.cjs
View file
Open in desktop
File renamed without changes.
0 packages/dds/tree/.mocharc.js → packages/dds/tree/.mocharc.cjs
View file
Open in desktop
File renamed without changes.
10 changes: 10 additions & 0 deletions packages/dds/tree/api-extractor-cjs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "../../../common/build/build-common/api-extractor-base.cjs.primary.json",
"compiler": {
"tsconfigFilePath": "<projectFolder>/tsconfig.cjs.json"
},
"apiReport": {
"enabled": false
}
Comment on lines +4 to +9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be defined in the base config? If not, add some comments to this file with some explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what the question is.

  • Should tsconfigFilePath be set in build-common/api-extractor-base.cjs*.json? No each package may have its own tsconfig setup at the moment.
  • Should apiReport be disabled in build-common/api-extractor-base.json? I think it should probably be off in there. The -base.*.primary.json files however would want it on. This config wants the extension neutral (.ts) settings from .primary.json but not the apiReport.

}
14 changes: 14 additions & 0 deletions packages/dds/tree/api-extractor-esm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "../../../common/build/build-common/api-extractor-base.esm.primary.json",
"compiler": {
"overrideTsconfig": {
"$schema": "http://json.schemastore.org/tsconfig",
"extends": "./tsconfig.esnext.json",
"compilerOptions": {
"module": "esnext",
"moduleResolution": "Bundler"
}
}
}
Comment on lines +4 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here.

}
2 changes: 1 addition & 1 deletion packages/dds/tree/api-extractor-lint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "../../../common/build/build-common/api-extractor-lint.json"
"extends": "../../../common/build/build-common/api-extractor-lint.esm.primary.json"
}
13 changes: 12 additions & 1 deletion packages/dds/tree/api-extractor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "../../../common/build/build-common/api-extractor-base.json"
"extends": "../../../common/build/build-common/api-extractor-base.json",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this extend from api-extractor-base while the others use different bases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? Seems like the package.json scripts explicitly pass a config file.

Copy link
Contributor Author

@jason-ha jason-ha Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Everything eventually extends from api-extractor-base. The other build-common files extend api-extractor-base, set entry point, set rollup files, and if not primary disable apiReport.
  • build:docs and ci:build:docs use this file implicitly.

"mainEntryPointFilePath": "./lib/index.d.ts",
"compiler": {
"overrideTsconfig": {
"$schema": "http://json.schemastore.org/tsconfig",
"extends": "./tsconfig.esnext.json",
"compilerOptions": {
"module": "esnext",
"moduleResolution": "Bundler"
}
}
}
}
46 changes: 43 additions & 3 deletions packages/dds/tree/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,42 @@
"license": "MIT",
"author": "Microsoft and contributors",
"sideEffects": false,
"type": "module",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a bit dangerous?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that this is dangerous. Why might it be dangerous?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I haven't requested the tsc-multi feature that I think it needs. Without tsc seeing "type": "module" it doesn't go into full ESM mode. This is a must for now to ensure proper ESM transpilation.)

"exports": {
".": {
"import": {
"types": "./lib/tree-public.d.ts",
jason-ha marked this conversation as resolved.
Show resolved Hide resolved
"default": "./lib/index.js"
},
"require": {
"types": "./dist/tree-public.d.ts",
"default": "./dist/index.js"
}
},
"./internal": {
"import": {
"types": "./lib/index.d.ts",
"default": "./lib/index.js"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
}
},
"main": "dist/index.js",
"module": "lib/index.js",
"types": "dist/index.d.ts",
"scripts": {
"api": "fluid-build . --task api",
"api-extractor:commonjs": "api-extractor run --config ./api-extractor-cjs.json",
"api-extractor:esnext": "api-extractor run --local --config ./api-extractor-esm.json",
"bench": "mocha --timeout 999999 --perfMode --parentProcess --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js",
"bench:profile": "mocha --v8-prof --v8-logfile=profile.log --v8-no-logfile-per-isolate --timeout 999999 --perfMode --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js && node --prof-process profile.log > profile.txt && rimraf profile.log && echo See results in profile.txt",
"build": "fluid-build . --task build",
"build:commonjs": "fluid-build . --task commonjs",
"build:compile": "fluid-build . --task compile",
"build:docs": "api-extractor run --local",
"build:esnext": "tsc --project ./tsconfig.esnext.json",
"build:genver": "gen-version",
jason-ha marked this conversation as resolved.
Show resolved Hide resolved
"build:test": "tsc --project ./src/test/tsconfig.json",
"check:are-the-types-wrong": "attw --pack . --entrypoints .",
"check:release-tags": "api-extractor run --local --config ./api-extractor-lint.json",
Expand All @@ -46,7 +70,7 @@
"test:snapshots:regen": "npm run test:mocha -- --snapshot",
"test:stress": "cross-env FUZZ_TEST_COUNT=20 FUZZ_STRESS_RUN=true mocha --ignore \"dist/test/memory/**/*\" --recursive \"dist/test/**/*.spec.js\" -r @fluidframework/mocha-test-setup",
"ts2esm": "ts2esm ./tsconfig.json ./src/test/tsconfig.json",
"tsc": "tsc",
"tsc": "tsc --project ./tsconfig.cjs.json && echo {\\\"module\\\": \\\"commonjs\\\"} > dist/package.json",
"typetests:gen": "fluid-type-test-generator",
"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
},
Expand Down Expand Up @@ -127,6 +151,22 @@
"ts2esm": "^1.1.0",
"typescript": "~5.1.6"
},
"fluidBuild": {
"tasks": {
"build:docs": {
"dependsOn": [
"build:esnext"
],
"script": false
},
"check:release-tags": [
"build:esnext"
],
"ci:build:docs": [
"build:esnext"
]
}
},
"typeValidation": {
"disabled": true,
"broken": {}
Expand Down
17 changes: 17 additions & 0 deletions packages/dds/tree/tsconfig.base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"extends": "@fluidframework/build-common/ts-common-config.json",
jason-ha marked this conversation as resolved.
Show resolved Hide resolved
"exclude": ["src/test/**/*"],
"compilerOptions": {
"rootDir": "./src",
"outDir": "./error-override-outdir-in-main-tsconfig",
"noImplicitAny": true,
"composite": true,
"preserveConstEnums": true,
// Allow unused locals.
// This is needed for type assertions using the TypeCheck library.
// Linter is used to enforce "_" prefix for unused locals to prevent accidentally having unused locals.
"noUnusedLocals": false,
"noImplicitOverride": true,
},
"include": ["src/**/*"],
}
7 changes: 7 additions & 0 deletions packages/dds/tree/tsconfig.cjs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "./tsconfig.base.json",
"compilerOptions": {
"outDir": "./dist",
"module": "CommonJS",
},
}
5 changes: 3 additions & 2 deletions packages/dds/tree/tsconfig.esnext.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"extends": "./tsconfig.json",
"extends": "./tsconfig.base.json",
"compilerOptions": {
"outDir": "./lib",
"module": "esnext",
"module": "Node16",
"moduleResolution": "Node16",
},
}
16 changes: 1 addition & 15 deletions packages/dds/tree/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,3 @@
{
"extends": "@fluidframework/build-common/ts-common-config.json",
"exclude": ["src/test/**/*"],
"compilerOptions": {
"rootDir": "./src",
"outDir": "./dist",
"noImplicitAny": true,
"composite": true,
"preserveConstEnums": true,
// Allow unused locals.
// This is needed for type assertions using the TypeCheck library.
// Linter is used to enforce "_" prefix for unused locals to prevent accidentally having unused locals.
"noUnusedLocals": false,
"noImplicitOverride": true,
},
"include": ["src/**/*"],
"extends": "./tsconfig.esnext.json",
}