-
Notifications
You must be signed in to change notification settings - Fork 361
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
fix: Allows naming of outputs individually #896
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 8aaf60f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
298e8a4
to
13cc452
Compare
13cc452
to
c9d5f24
Compare
? blue(`Build "${options.pkg.name}" to ${targetDir}:`) | ||
? blue(`Built "${options.pkg.name}":`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targetDir
is a bit unknowable now (each output format could theoretically have a different output directory, if the user so desired), so it's been removed.
@@ -279,42 +270,45 @@ function walk(exports, includeDefault) { | |||
function getMain({ options, entry, format }) { | |||
const { pkg } = options; | |||
const pkgMain = options['pkg-main']; | |||
const pkgTypeModule = pkg.type === 'module'; | |||
let multipleEntries = options.multipleEntries; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multipleEntries
needs to act as an escape hatch to our name resolution logic to ensure no collisions.
This can be seen with Microbundle itself; when we create a cli.js
and microbundle.js
, we need to ensure the assigned name of cli
does not get replaced by the "main"
field ("microbundle").
mainNoExtension, | ||
mainsByFormat.es = resolve( | ||
(!multipleEntries && | ||
(pkg.module && !pkg.module.match(/src\//) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random q: What's this match against src/
about? Goes back to repo init afaict so couldn't find any context there.
c9d5f24
to
7402b06
Compare
7402b06
to
9730b95
Compare
Size Change: 0 B Total Size: 65.3 kB ℹ️ View Unchanged
|
9730b95
to
8aaf60f
Compare
Summary
Closes #884 and #825
Each output can be individually given output paths now, there's no sharing of names/paths. This removes an over reliance upon
"main"
to provide output paths for every other format.There's no longer a guarantee of all output being in the same directory: based on the file paths, different formats can be output in different locations.
The
--output
flag is still functional, but only covers formats that are not provided an explicit path in thepackage.json
. For example, if we're missing"module"
but still outputting ESM, it will be output at<--output>/<pkg-name>.esm.{js/mjs}
For output formats that are not given an explicit path/name, they default to the package name. I think this or
index
are equally valid, but this a) matches our examples in the ReadMe (if we have an example module named "foo", the output tends to befoo.js
,foo.esm.js
, etc.) and b) real world usage. People tend to prefer naming output than keeping it asindex
from what I've seen, but YMMV.Edit: Both #825 and #938 bring up the filename specified with
"types"
not being acknowledged and I don't think it's something we can support. TSC itself outputs filenames as-is, there's no easy way (besides a post-build script) to have it instead output an arbitrary name for the entry file.rpt2
forgoes the Rollup process anyhow under certain circumstances and uses TS to write directly to the filesystem, so we don't necessarily have a good way of altering that post-build ourselves I don't think.