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
#2585 - Feature adds ability to use a function that returns a pattern string in all places where you could use a pattern string befor. #3658
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3658 +/- ##
=======================================
Coverage 96.70% 96.70%
=======================================
Files 183 183
Lines 6287 6291 +4
Branches 1832 1833 +1
=======================================
+ Hits 6080 6084 +4
Misses 105 105
Partials 102 102
Continue to review full report at Codecov.
|
@lukastaegert I think it is now ready for merge I did add a test it works well and I cleaned the code maybe we need to move the test but I think it is ok for now. |
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.
Thanks for this PR. Besides the comments I made, two things need to be done:
- The public TypeScript types in
src/rollup/types.d.ts
need to be updated to reflect the extended interface - The documentation in
docs/999-big-list-of-options.md
needs to be updated
I can take care of the latter if you want if the rest is in place.
Better Implementation now with a test Add: Tests Fix: License Now passing a object { replacements, meta() } Add: docs and renamed property
@lukastaegert Usagerollup.config.js function debug(v,replacements,getFileInfo) {
console.log(v)
console.log(getFileInfo())
console.log(replacements)
return v
}
module.exports = {
input: ['main.js'],
plugins: {
transform() {
this.emitFile({ type: 'asset', name: 'test.txt', source: 'hello world' });
return null;
}
},
output: {
entryFileNames: ({ replacements, getFileInfo }) => debug(`[name].js`,replacements,getFileInfo),
assetFileNames: ({ replacements, getFileInfo }) => debug('[ext]/[hash]-[name][extname]',replacements,getFileInfo),
chunkFileNames: ({ replacements, getFileInfo }) => debug('chunk-[name]-[hash]-[format].js',replacements,getFileInfo)
}
} //assetFileNames [ext]/[hash]-[name][extname]
{
name: 'test.txt',
source: 'hello world',
output: {
assetFileNames: [Function: assetFileNames],
bundle: [Object: null prototype] {}
}
}
{
hash: [Function: hash],
ext: [Function: ext],
extname: [Function: extname],
name: [Function: name]
}
//entryFileNames [name].js
{
code: undefined,
dynamicImports: [ null ],
exports: [],
facadeModuleId: '/home/frank/Projekte/rollup/test/chunking-form/samples/emit-file/filenames-function-patterns/main.js',
fileName: undefined,
implicitlyLoadedBefore: [],
imports: [],
isDynamicEntry: false,
isEntry: true,
isImplicitEntry: false,
map: undefined,
modules: [Object: null prototype] {
'/home/frank/Projekte/rollup/test/chunking-form/samples/emit-file/filenames-function-patterns/main.js': {
originalLength: 54,
removedExports: [],
renderedExports: [],
renderedLength: 55
}
},
name: [Getter],
type: 'chunk'
}
{
format: [Function: format],
hash: [Function: hash],
name: [Function: name]
}
// chunkFileName chunk-[name]-[hash]-[format].js
{
code: undefined,
dynamicImports: [],
exports: [ 'default' ],
facadeModuleId: '/home/frank/Projekte/rollup/test/chunking-form/samples/emit-file/filenames-function-patterns/deb.js',
fileName: undefined,
implicitlyLoadedBefore: [],
imports: [],
isDynamicEntry: true,
isEntry: false,
isImplicitEntry: false,
map: undefined,
modules: [Object: null prototype] {
'/home/frank/Projekte/rollup/test/chunking-form/samples/emit-file/filenames-function-patterns/deb.js': {
originalLength: 23,
removedExports: [],
renderedExports: [Array],
renderedLength: 19
}
},
name: [Getter],
type: 'chunk'
}
{
format: [Function: format],
hash: [Function: hash],
name: [Function: name]
} |
While these offer many possibilities, I think it should not really be necessary to add both the prerendered chunk info and the property infos considering you can still use the pattern placeholders to get those. I added a commit that simplifies the interface so get chunk info is always called when you use a function. I said it is slightly expensive, but not that expensive. I just wanted to avoid generating this information when we are sure it will not be used. But requiring the user to call a function feels like a cumbersome interface for me. I also slightly extended the test to assert what the parameters are, extended the types everywhere and extended the documentation so that it should be clear what the parameters are. |
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.
From my side, this can be merged and released now, but please have another look.
LGTM +1 from my side we can improve that over time i just wanted to do the google guys a favor so that they can rank rollup better on tools.report they did create a list of issues in general rollup offers everything but it needs some documentation and examples maybe and the t3 refactoring that we could also need for a better cache implementation |
definitely. I studied this list and that one was definitely the low-hanging fruit, thanks for pushing this forward, not sure when I would have gotten to it otherwise. |
This is great to see. Give me a nudge when this lands and I'll update tooling.report. |
@jakearchibald you commented already after it landed :) |
Doh, well, just goes to show how happy I am about this 😄 |
Ah, but I forgot to push the docs update to the website. Done now, here it is: https://rollupjs.org/guide/en/#outputentryfilenames |
Closes: #2585
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
Adds the ability to use a function that accepts a string as argument and returns a string with a pattern to get used in every fild that supports patterns. see #2585