[INFRA] Split build output directories #1004

tbouffard · 2021-01-05T07:06:33Z

Previously, everything was built into the 'dist' folder so it was hard to know
which build part generated which files.
So, the output directories have been split:

Non bundled Javascript: build/lib (only generated when running the tsc command)
Demo: build/demo
Development: build/public
Bundles: dist (as before)

In addition, the gitignore, eslintignore and clean script have been simplified to remove
output folders that are not generated anymore (coverage related).
The tsconfig.json has been simplified: remove extra configurations only run by tsc. We
don't generate sourceMap nor declaration files as they are only useful for bundles (declaration)
or when running the development server (sourceMap).

Previously, everything was built into the 'dist' folder so it was hard to know which build part generated which files. So, the output directories have been split: - Non bundled Javascript: lib (only generated when running the tsc command) - Demo: build/demo - Development: build/public - Bundles: dist (as before) In addition, the gitignore and clean script have been simplified to remove output folders that are not generated anymore (coverage related).

…he server

csouchet · 2021-01-08T14:15:05Z

tsconfig.json

@@ -1,8 +1,8 @@
 {
  "compilerOptions": {
    "declaration": true,
-    "declarationDir": "./dist",
-    "outDir": "./dist",
+    "declarationDir": "./lib",


Is it normal that the generated files for the type definition are in the dist folder ?
I don't have 'lib' generated folder.

Files are generated in the lib directory only when running with the tsc command or build triggered by IDE (like IntelliJ).
When running with rollup, the typescript plugin seems generating the definition files in the same folder as the one where the bundle is generated, so the dist folder.

I have changed the tsconfig.json because I would like we provide a non bundled version of the ESM package. We could use the lib folder to package it in the npm package.

For me, it's weird to have the generated files not at the same place if we us rollup or tsc.

@aibcmars A idea ?

at first glance it is strange indeed
I have started to check it up locally - should be able to tell something more soon

Just remember that rollup generates bundles (single javascript files with all the lib) and the purpose of this PR is to split the output directories based on usage of the generated resources. Mainly to avoid the current mess we have and which requires cleaning: we have everything in the dist folder, we don't know where the files come from (see commit and PR description).

The dev bundle for the demo/development is now generated in a subfolder of the build and the dev http server uses it.
The bundles for the npm package stay in the dist folder to avoid package breaking change.
We have each javascript generated files from each typescript files in the lib folder. We may in the future use them directly to generate the bundles without having to rebuild them with the typescript plugin. This is what roughjs does for instance: https://github.com/rough-stuff/rough/blob/v3.1.0/package.json#L10

split is a great idea 👍
bundles ok to have it under /dist 👍

however, I do not really see the interest in having the additional /lib directory

--- edit
OK, I see now your previous comment @tbouffard
so it is just to provide types for simple esm 👍

I am still wondering?
[in the case for not bundled esm]

Isn't it enough to just have the definitions under /dist

We could talk about this directly on next week.

As having a lib folder may only be useful in case of a non bundled ESM module (not done for now), we could generate the single javascript files in build/lib or build/bin to hide them.
For the declaration files, we can store them wherever we want as far as we reference the right path in the package.json types field.

We (@csouchet, @aibcmars and @tbouffard) have decided to

store the tsc outputs to build/lib

don't generate declarations when running tsc as they are never used

csouchet

When I run the command npm run lint (after running npm run build-utils), I have the following error:

...../BPMN-Visualization/scripts/utils/dist/src/model/bpmn/json/Semantic.d.ts
  25:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  26:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  27:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface

✖ 3 problems (3 errors, 0 warnings)

I think that something is missing in .eslintignore file, or the output of utils.rollup.config.js is wrong.

tbouffard · 2021-01-08T14:49:20Z

When I run the command npm run lint (after running npm run build-utils), I have the following error:
...../BPMN-Visualization/scripts/utils/dist/src/model/bpmn/json/Semantic.d.ts
  25:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  26:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  27:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface

✖ 3 problems (3 errors, 0 warnings)
I think that something is missing in .eslintignore file, or the output of utils.rollup.config.js is wrong.

I have reproduced the issue with the master branch (commit a1760c5), so it seems not related to the changes proposed here.
@csouchet do you want us to create an issue for this?

csouchet · 2021-01-08T14:59:28Z

When I run the command npm run lint (after running npm run build-utils), I have the following error:
...../BPMN-Visualization/scripts/utils/dist/src/model/bpmn/json/Semantic.d.ts
  25:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  26:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface
  27:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface

✖ 3 problems (3 errors, 0 warnings)
I think that something is missing in .eslintignore file, or the output of utils.rollup.config.js is wrong.
I have reproduced the issue with the master branch (commit a1760c5), so it seems not related to the changes proposed here.
@csouchet do you want us to create an issue for this?

👍

I think we should create a issue 🙂 --> done in #1020

# Conflicts: # .eslintignore

As tsc is only run by IDE and lint command disable declaration and sourceMap files generation as they are not useful in this case.

package.json

# Conflicts: # test/e2e/helpers/visu/PageTester.ts

aibcmars

LGTM

sonarqubecloud · 2021-01-12T13:40:41Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

tbouffard added the chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...) label Jan 5, 2021

tbouffard marked this pull request as draft January 5, 2021 07:28

visual test: fix path where diagrams are copied to be available for t…

4e7cf9c

…he server

tbouffard marked this pull request as ready for review January 5, 2021 09:35

tbouffard mentioned this pull request Jan 7, 2021

[TEST] Add automatic bundle tests #1010

Merged

Merge branch 'master' into infra/split_output_directories

4255c0b

csouchet reviewed Jan 8, 2021

View reviewed changes

Merge branch 'master' into infra/split_output_directories

13a676d

csouchet and others added 2 commits January 8, 2021 16:14

Merge branch 'master' into infra/split_output_directories

811d9d6

Merge branch 'master' into infra/split_output_directories

40d85e6

tbouffard mentioned this pull request Jan 11, 2021

[INFRA] Lint error after building utils #1020

Closed

tbouffard added 3 commits January 11, 2021 18:01

Update .eslintignore with new output directories

7f47135

Merge branch 'master' into infra/split_output_directories

01fa3dd

# Conflicts: # .eslintignore

tsc outputs to build/lib

8a319e8

As tsc is only run by IDE and lint command disable declaration and sourceMap files generation as they are not useful in this case.

tbouffard requested review from csouchet and aibcmars January 12, 2021 10:00

aibcmars reviewed Jan 12, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

csouchet approved these changes Jan 12, 2021

View reviewed changes

csouchet added the rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged label Jan 12, 2021

tbouffard added 3 commits January 12, 2021 14:29

package.json: remove 'lib' directory from the clean script

46891c5

Merge branch 'master' into infra/split_output_directories

1ab1cad

# Conflicts: # test/e2e/helpers/visu/PageTester.ts

Fix merge: path where to copy diagrams for visual tests

aae959f

aibcmars approved these changes Jan 12, 2021

View reviewed changes

tbouffard merged commit 4bde056 into master Jan 12, 2021

tbouffard deleted the infra/split_output_directories branch January 12, 2021 13:40

tbouffard removed the rebase needed 💥 Pull request that must be rebased on the latest master commit prior being reviewed or merged label May 11, 2021

tbouffard mentioned this pull request May 11, 2021

[INFRA] Restore Typescript sourcemap for development #1294

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INFRA] Split build output directories #1004

[INFRA] Split build output directories #1004

tbouffard commented Jan 5, 2021 •

edited

Loading

csouchet Jan 8, 2021

tbouffard Jan 8, 2021

tbouffard Jan 8, 2021 •

edited

Loading

csouchet Jan 8, 2021

aibcmars Jan 8, 2021

tbouffard Jan 8, 2021

aibcmars Jan 8, 2021 •

edited

Loading

aibcmars Jan 8, 2021

tbouffard Jan 8, 2021

tbouffard Jan 12, 2021

csouchet left a comment

tbouffard commented Jan 8, 2021

csouchet commented Jan 8, 2021 •

edited by tbouffard

Loading

aibcmars left a comment

sonarqubecloud bot commented Jan 12, 2021

[INFRA] Split build output directories #1004

[INFRA] Split build output directories #1004

Conversation

tbouffard commented Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbouffard Jan 8, 2021 • 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

aibcmars Jan 8, 2021 • 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

csouchet left a comment

Choose a reason for hiding this comment

tbouffard commented Jan 8, 2021

csouchet commented Jan 8, 2021 • edited by tbouffard Loading

aibcmars left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 12, 2021

tbouffard commented Jan 5, 2021 •

edited

Loading

tbouffard Jan 8, 2021 •

edited

Loading

aibcmars Jan 8, 2021 •

edited

Loading

csouchet commented Jan 8, 2021 •

edited by tbouffard

Loading