#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

frank-dspeed · 2020-07-04T10:17:43Z

Closes: #2585

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

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

codecov · 2020-07-04T10:23:55Z

Codecov Report

Merging #3658 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           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

Impacted Files	Coverage Δ
src/Bundle.ts	`100.00% <ø> (ø)`
src/utils/PluginDriver.ts	`100.00% <ø> (ø)`
src/Chunk.ts	`100.00% <100.00%> (ø)`
src/utils/FileEmitter.ts	`100.00% <100.00%> (ø)`
src/utils/renderNamePattern.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8339c37...6df06e7. Read the comment docs.

frank-dspeed · 2020-07-04T11:33:19Z

@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.

lukastaegert

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.

LICENSE.md

src/utils/renderNamePattern.ts

test/misc/output-options-pattern-function.js

# Das ist die erste Commit-Beschreibung: #2585 # Das ist Commit-Beschreibung #2: Better Implementation now with a test # Das ist Commit-Beschreibung #3: Add: Tests # Das ist Commit-Beschreibung #4: Fix: License

Better Implementation now with a test Add: Tests Fix: License Now passing a object { replacements, meta() } Add: docs and renamed property

frank-dspeed · 2020-07-05T14:05:49Z

@lukastaegert
this are the objects now that you can use in the pattern function how should i document that ? or should we even? I think that needs a section some where. Or maybe i should do examples?

Usage

rollup.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]
}

lukastaegert · 2020-07-06T06:18:48Z

this are the objects now that you can use

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.

lukastaegert

From my side, this can be merged and released now, but please have another look.

frank-dspeed · 2020-07-06T12:06:06Z

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

lukastaegert · 2020-07-06T18:34:17Z

so that they can rank rollup better on tools.report

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.

jakearchibald · 2020-07-07T11:23:46Z

This is great to see. Give me a nudge when this lands and I'll update tooling.report.

frank-dspeed · 2020-07-07T15:35:06Z

@jakearchibald you commented already after it landed :)

jakearchibald · 2020-07-07T15:38:07Z

Doh, well, just goes to show how happy I am about this 😄

lukastaegert · 2020-07-07T18:11:39Z

Ah, but I forgot to push the docs update to the website. Done now, here it is: https://rollupjs.org/guide/en/#outputentryfilenames

This was referenced Jul 4, 2020

output.entryFileNames - accept a function? #2585

Closed

Add support for file extension placeholder and function in output.entryFileNames option #3107

Closed

frank-dspeed changed the title ~~https://github.com/rollup/rollup/issues/2585~~ #2585 - Feature adds ability to use a function that returns a pattern string in all places where you could use a pattern string befor. Jul 4, 2020

lukastaegert requested changes Jul 5, 2020

View reviewed changes

LICENSE.md Outdated Show resolved Hide resolved

src/utils/renderNamePattern.ts Outdated Show resolved Hide resolved

test/misc/output-options-pattern-function.js Outdated Show resolved Hide resolved

frank-dspeed added 6 commits July 5, 2020 15:04

# Das ist eine Kombination aus 4 Commits.

4f514eb

# Das ist die erste Commit-Beschreibung: #2585 # Das ist Commit-Beschreibung #2: Better Implementation now with a test # Das ist Commit-Beschreibung #3: Add: Tests # Das ist Commit-Beschreibung #4: Fix: License

https://github.com/rollup/rollup/issues/2585

3d68d97

Better Implementation now with a test Add: Tests Fix: License Now passing a object { replacements, meta() } Add: docs and renamed property

Corrected types

d40ca6a

add: test

ca95e7a

Upgraded test to include a chunk via dynamic import

4153856

Fix: solo test

1f32601

Refine interfaces, types, docs and test

a0e4469

Further refine docs

6df06e7

lukastaegert approved these changes Jul 6, 2020

View reviewed changes

lukastaegert merged commit 19e50af into rollup:master Jul 6, 2020

frank-dspeed mentioned this pull request Jul 8, 2020

Naming dynamic chunks #2793

Closed

developit mentioned this pull request Jul 11, 2020

Switch to parametric output.*FileNames preactjs/wmr#58

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#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

#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

frank-dspeed commented Jul 4, 2020 •

edited

codecov bot commented Jul 4, 2020 •

edited

frank-dspeed commented Jul 4, 2020

lukastaegert left a comment

frank-dspeed commented Jul 5, 2020 •

edited

lukastaegert commented Jul 6, 2020

lukastaegert left a comment

frank-dspeed commented Jul 6, 2020

lukastaegert commented Jul 6, 2020

jakearchibald commented Jul 7, 2020

frank-dspeed commented Jul 7, 2020

jakearchibald commented Jul 7, 2020

lukastaegert commented Jul 7, 2020

#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

#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

frank-dspeed commented Jul 4, 2020 • edited

Description

codecov bot commented Jul 4, 2020 • edited

Codecov Report

frank-dspeed commented Jul 4, 2020

lukastaegert left a comment

Choose a reason for hiding this comment

frank-dspeed commented Jul 5, 2020 • edited

Usage

lukastaegert commented Jul 6, 2020

lukastaegert left a comment

Choose a reason for hiding this comment

frank-dspeed commented Jul 6, 2020

lukastaegert commented Jul 6, 2020

jakearchibald commented Jul 7, 2020

frank-dspeed commented Jul 7, 2020

jakearchibald commented Jul 7, 2020

lukastaegert commented Jul 7, 2020

frank-dspeed commented Jul 4, 2020 •

edited

codecov bot commented Jul 4, 2020 •

edited

frank-dspeed commented Jul 5, 2020 •

edited