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

Use @wessberg/rollup-plugin-ts + change output naming structure #219

Conversation

tunnckoCore
Copy link

@tunnckoCore tunnckoCore commented Sep 30, 2019

Addresses #208 and #200.

  • Switch the output naming structure/convention. We are kind of forced to do that to make a better and more predictable experience, and integrating this plugin.
  • Update tsdx create according to new output structure; Plus, correct "package generation". Currently, if user type @hela/core in the prompt it will create core folder in the cwd and also will add to the generated package.json file a name field to be core instead @hela/core, which definitely isn't correct behavior.
  • Remove the "utils safePackageName" test.
  • Remove the safePackageName function, at all.
  • ESLint autofix is enabled on my VSCode, so ESLint removed the use strict on the tsdx-build.test.js, obviously according to the given eslint config and prettier.
  • Immediately before I started anything with just pnpm i on fresh branch, the pnpm's lockfile changed.

The only left is to update the babelPluginTsdx file, meaning to drop the rollup-plugin-babel dependency and just generate babel config and pass it as babelConfig option to the ts plugin. But keep in mind the ts plugin also automatically includes some plugins and does merging some default options.

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore tunnckoCore changed the title Use @wessberg/rollup-plugin-ts Use @wessberg/rollup-plugin-ts + change output naming structure Sep 30, 2019
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@jaredpalmer
Copy link
Owner

Why are we changing the output format?

Copy link
Owner

@jaredpalmer jaredpalmer left a comment

Choose a reason for hiding this comment

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

I would like to keep the dev/prod distinction in cjs. Also this is breaking change

`${paths.appDist}/${safePackageName(opts.name)}`,
opts.format,
opts.env,
`${paths.appDist}/${opts.format}/index`,
Copy link
Owner

Choose a reason for hiding this comment

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

If we were nice we would automagically update folks package.json for them

Copy link
Author

Choose a reason for hiding this comment

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

Meaning? Codemod thingy? It's could be the next possible step after this whole change.


expect(shell.test('-f', 'dist/index.d.ts')).toBeTruthy();
expect(shell.test('-f', 'dist/cjs/index.js')).toBeTruthy();
expect(shell.test('-f', 'dist/cjs/index.min.js')).toBeTruthy();
Copy link
Owner

Choose a reason for hiding this comment

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

I am confused. Is .min the production version?

Copy link
Author

Choose a reason for hiding this comment

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

Sure. Why otherwise one will want minifying? and especially, in cjs/node target.

@tunnckoCore
Copy link
Author

Why are we changing the output format?

For "reasons", the other PR has comments around that :) Or at least because such design decisions, problems and "features" appear in some other places/tools (referring the new "hook" option in the ts plugin). All this is just one simple example that may seem negligible, but this is happening for years in the ecosystem.

@agilgur5 agilgur5 added topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 topic: v0.10.x Issues related to broken builds in v0.10.x / @wessberg/rollup-plugin-ts labels Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 topic: v0.10.x Issues related to broken builds in v0.10.x / @wessberg/rollup-plugin-ts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants