[v2.0.0] Re design of the plugin #61
Conversation
yoannmoinet
force-pushed
the
yoann/re-design
branch
from
189e682 to
9c17cd0
Compare
| "license": "MIT", | ||
| "private": true, | ||
| "author": "Datadog", | ||
| "exports": { |
There was a problem hiding this comment.
FYI (not blocking): instead of using exports for such static files, you can use files to mark the src/ folder as public
Or even don't write anything, and keep all the files top level in this package
There was a problem hiding this comment.
I wanted to keep the "accessible" files separated from the "meta" config of the package (tsconfig, package.json, etc...).
Only way I could find was to use a src/ folder for actually reachable files.
There was a problem hiding this comment.
package.json should always be accessible
And if you want to remove those tsconfig and stuff, you can use either files in the package.json to opt in some, or use .npmignore to remove some on publish, see https://docs.npmjs.com/cli/v10/configuring-npm/package-json#files
| @@ -0,0 +1 @@ | |||
| # Core | |||
There was a problem hiding this comment.
You can remove this file as it doesn't add that much value
There was a problem hiding this comment.
I'll fill it out in a different PR where I'll go around all docs.
There was a problem hiding this comment.
LGTM
One thing I just realized: in your exports, you referenced the .ts files. This will work in dev, but I don't think it'll work in prod when the package will be released as those files won't exist anymore
And this can break libraries / codebases using this library
packages/core/package.json
Outdated
| "scripts": { | ||
| "typecheck": "tsc --noEmit" | ||
| "build": "yarn clean; tsc", |
There was a problem hiding this comment.
shouldn't this be yarn clean && tsc?
There was a problem hiding this comment.
Seems to give the same result.
packages/esbuild-plugin/package.json
Outdated
| "peerDependenciesMeta": { | ||
| "esbuild": { | ||
| "optional": true | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it still optional? (as this is the esbuild-plugin)
There was a problem hiding this comment.
Good point, will update.
packages/webpack-plugin/package.json
Outdated
| "peerDependenciesMeta": { | ||
| "webpack": { | ||
| "optional": true | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it still optional as thi sis the webpack plugin?
There was a problem hiding this comment.
Good point, will update.
packages/webpack-plugin/package.json
Outdated
| "webpack": "5.49.0" | ||
| }, | ||
| "peerDependencies": { | ||
| "webpack": "*" |
There was a problem hiding this comment.
nit: maybe we want to say 5.x.x as Idk if this works with webpack 4 (and def. doesn't work with webpack 3)
There was a problem hiding this comment.
Good point, it should support webpack 4 and 5.
I'll update.
yoannmoinet
force-pushed
the
yoann/re-design
branch
from
4a5e529 to
0bc0d61
Compare
What and why?
This PR redesigns how this plugin is implemented to facilitate the composition we'll need to do later on.
How?
We move to a packages approach using yarn workspaces.
@datadog/build-plugins-assets|./packages/assets: for all the static assets we may use.@datadog/build-plugins-core|./packages/core: for everything that is re-used somewhere else.@datadog/esbuild-plugin|./packages/esbuild-plugin: for the esbuild plugin.@datadog/build-plugins-tests|./packages/tests: for all the tests we have.project-esbuild|./packages/tests/mocks/projects/esbuild: for a mock project built with esbuild.project-webpack4|./packages/tests/mocks/projects/webpack4: for a mock project build with webpack 4.project-webpack5|./packages/tests/mocks/projects/webpack5: for a mock project build with webpack 5.@datadog/build-plugins-tools|./packages/tools: for the various tools we use on the infra side, aka a CLI.@datadog/webpack-plugin|./packages/webpack-plugin: for the webpack plugin.Everything else in the PR is about updating dependencies and make everything work together.
Fixes #60