implement `amd.autoId`/`amd.basePath` options #3867

tjenkinson · 2020-11-13T16:11:40Z

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

Related issues: #3333

Description

It's currently possible to set an amd.id, but when there are multiple chunks they all get the same id, which isn't helpful.

This PR implements idFromChunkName, which when true causes amd.id to become a prefix, and the chunk name is then used in the id, so that the chunk filename matches the id in the AMD id. This makes it possible to use AMD ids for builds that introduce code splitting.

To achieve this the chunk id is now passed to the finalizers.

tjenkinson · 2020-11-13T16:12:14Z

src/Chunk.ts

@@ -719,6 +719,7 @@ export default class Chunk {
 				dependencies: [...this.renderedDependencies!.values()],
 				exports: this.renderedExports!,
 				hasExports,
+				id: this.id as string,


Am I correct assuming id is always a string at this point?

At this point, yes.

codecov · 2020-11-13T16:30:54Z

Codecov Report

Merging #3867 (cbae7b8) into master (0786839) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3867   +/-   ##
=======================================
  Coverage   97.07%   97.07%           
=======================================
  Files         185      187    +2     
  Lines        6523     6539   +16     
  Branches     1889     1901   +12     
=======================================
+ Hits         6332     6348   +16     
  Misses        101      101           
  Partials       90       90

Impacted Files	Coverage Δ
src/finalisers/index.ts	`100.00% <ø> (ø)`
src/utils/renderNamePattern.ts	`100.00% <ø> (ø)`
src/Bundle.ts	`100.00% <100.00%> (ø)`
src/Chunk.ts	`100.00% <100.00%> (ø)`
src/finalisers/amd.ts	`100.00% <100.00%> (ø)`
src/finalisers/shared/getCompleteAmdId.ts	`100.00% <100.00%> (ø)`
...alisers/shared/removeExtensionFromRelativeAmdId.ts	`100.00% <100.00%> (ø)`
src/finalisers/shared/removeJsExtension.ts	`100.00% <100.00%> (ø)`
src/finalisers/umd.ts	`100.00% <100.00%> (ø)`
src/utils/FileEmitter.ts	`100.00% <100.00%> (ø)`
... and 3 more

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 0786839...cbae7b8. Read the comment docs.

lukastaegert

Thanks, I will need to test this thoroughly. So this looks like a very good start, I am mostly wondering about the interface and interaction with the amd.id option.

In a way, it feels weird to have this special option where we could integrate it into the amd.id option, e.g. via a placeholder pattern similar to what we have for the various filenames. Just prepending the id seems—arbitrary to me.

Another point we might add to this PR is to finally add a warning if a build produces several chunks with the same id, telling the user how to fix it.

Another thing I am wondering about: Isn't the whole point of using ids to be able to import files differently? I.e. shouldn't files be imported by their id instead of their file names if I am using ids? Because if you are using the ids, then you can concatenate the files and it will still work while if you are using file names, it will not be able to import the files.

We can actually write tests for this using the runAmd option you can use in chunking-form tests. This will actually run the code-split built against requirejs.

So we could write two tests:

If I am using file names as ids, does everything still work? (there should not be an issue at the moment, I guess)
If I am using arbitrary ids but concatenate the files in a generateBundle hook (i.e. delete everything from the bundle and emit a new file that concatenates everything as main.js, because that is what runAmd is looking for), does it still work?

lukastaegert · 2020-11-15T06:49:15Z

src/Chunk.ts

@@ -719,6 +719,7 @@ export default class Chunk {
 				dependencies: [...this.renderedDependencies!.values()],
 				exports: this.renderedExports!,
 				hasExports,
+				id: this.id as string,


At this point, yes.

lukastaegert · 2020-11-15T06:50:00Z

src/finalisers/amd.ts

@@ -5,14 +5,15 @@ import { getExportBlock, getNamespaceMarkers } from './shared/getExportBlock';
 import getInteropBlock from './shared/getInteropBlock';
 import warnOnBuiltins from './shared/warnOnBuiltins';

+function removeExtension(name: string) {


Maybe call this removeJsExtension, the name confused be as you definitely want to keep non-.js extensions!

tjenkinson · 2020-11-15T09:31:07Z

I was wondering how we could do it in a non breaking way.

a placeholder pattern similar to what we have for the various filenames

Sounds good, or maybe a callback if interpreting characters are a placeholder would be breaking.

add a warning if a build produces several chunks with the same id, telling the user how to fix it.

👍

Isn't the whole point of using ids to be able to import files differently?

I think for dynamic requires the default behaviour is to build the url to the file from the ID. We concat all files together that are in our entry point (one of which is the entry amd file from rollup), so they need unique ids for that.

We can actually write tests for this using the runAmd option you can use in chunking-form tests. This will actually run the code-split built against requirejs.

👍

tjenkinson · 2020-11-15T12:13:26Z

Pushed a commit to switch to a pattern. Just realised the umd finaliser will also need updating. Also if feels a bit strange that [id] is not just the id but is the id with '.js' removed.

lukastaegert · 2020-11-16T05:49:51Z

test/chunking-form/samples/amd-id-code-split/_config.js

+		}
+	},
+	additionalFormats: ['umd'],
+	runAmd: exports => {


This does not call the function. The correct syntax is

{ runAmd: { exports(exports) { // do something with exports } } }

You will find that exports is undefined for you. The reason is that indeed even though it loads the main chunk, it does not load the main module because the id does not match. If you add logs to your code, you will see that no code is executed.

To fix this, we should probably extend the runAmd object so that you can specify also the id of the main entry if it is not main, but it will require some more digging how to handle this in requirejs.

test/chunking-form/index.js

lukastaegert · 2020-11-17T06:23:05Z

I spend some more time digging into requirejs, so here is a revised version of the test loader that allows you to specify an alternative entry id via

runAmd: {
  id: 'something/main',
  // ... exports
}

The new test runner will still try to load the main file, but after the main file, it attempts to load the alternative id specified. This works if main.js contains a define call for the alternative id. If this does not work, an error is thrown, showing what ids were actually defined.

async function generateAndTestBundle(bundle, outputOptions, expectedDir, config) {
	await bundle.write(outputOptions);
	if (outputOptions.format === 'amd' && config.runAmd) {
		const entryAmdId = config.runAmd.id || entryFile;
		const requirejs = require('requirejs');
		requirejs.config({ baseUrl: outputOptions.dir });
		global.assert = require('assert');
		requirejs(entryFile);
		const exports = requirejs(entryAmdId);
		if (typeof exports === 'undefined') {
			throw new Error(
				`Entry AMD id "${entryAmdId}" was not defined by entry file "${entryFile}", registered ids: ${Object.keys(
					global.requirejsVars.requirejs.s.contexts._.registry
				)
					.map(id => `"${id}"`)
					.join(', ')}.`
			);
		}
		if (config.runAmd.exports) {
			await config.runAmd.exports(exports);
		}
		delete require.cache[require.resolve('requirejs')];
		delete global.requirejsVars;
		delete global.assert;
	}
	assertDirectoriesAreEqual(outputOptions.dir, expectedDir);
}

Playing around more, this confirmed my suspicion: If you keep using the chunk file names as ids when importing files (as your current implementation does), the actual code will just not be executed. It still works if you use [id] as the id, but fails as soon as you add something else.

But I could confirm that everything works as expected (at least if you manually concatenate for ids other than [id]) if you do not use the chunk id when importing but directly the chunk AMD id. And if you are using the AMD id, you do not need to turn imports into relative imports.

So assume you have chunks

main.js
nested/a.js
nested/b.js

and we assume that you use AMD ids [id]. Then nested/a.js can be loaded from main.js via import('nested/a') while nested/b.js can be loaded from nested/a.js via import('nested/b'). And this works both if you keep the chunks separate or if you concatenate everything into main.js.

So what needs to be done to make this PR functional is to use the chunks AMD.id instead of the file name, and this needs to happen outside the finalizers as part of the regular Chunk rendering process.

Depending on how this is done, one could consider moving the remaining AMD id logic out of the finalizers as well and just make the AMD id another parameter to pass in.

lukastaegert · 2020-11-17T06:25:50Z

test/chunking-form/index.js

@@ -10,7 +10,7 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
 		() => {
 			let bundle;

-			for (const format of FORMATS) {
+			for (const format of [...FORMATS, ...(config.additionalFormats || [])]) {


I know you use this to allow UMD here but really I do not think it makes a lot of sense considering UMD just does not support code-splitting. Instead, use a regular form test for the non-code-splitting case.

lukastaegert · 2020-11-17T06:26:55Z

test/chunking-form/samples/amd-id-code-split/_config.js

@@ -0,0 +1,17 @@
+module.exports = {


Your test names are switched: The code-split test is not doing the code splitting while the other one does

tjenkinson · 2020-11-17T09:52:23Z

So just to confirm I'm understanding. With the example

main.js
nested/a.js
nested/b.js

if the complete ids were main, nested/a and nested/b, then everything works.

It still works if you use [id] as the id, but fails as soon as you add something else.

Yeh this makes me think making it templateable isn't super useful?

Initially I supported a prefix because the output from rollup may be placed in a nested directory (which happens in our current setup), so the above output from rollup would end up as something like

rollup-bundle/main.js
rollup-bundle/nested/a.js
rollup-bundle/nested/b.js

meaning now, the complete ids would need to be rollup-bundle/main, rollup-bundle/nested/a, rollup-bundle/nested/b.

So what needs to be done to make this PR functional is to use the chunks AMD.id instead of the file name, and this needs to happen outside the finalizers as part of the regular Chunk rendering process.

Not following this. Using the amd.id is what happens now, which doesn't take into consideration the filename?

lukastaegert · 2020-11-18T05:26:40Z

What I want to say is:

if you use a prefix, it will no longer work with separate files
but it can be made to work if everything is concatenated

And I think the latter IS useful. But to make it work, one has to switch to using AMD ids instead of file names for all imports as well as adding the AMD ids to the rendered output. So far, you only did the latter.

If you think this is too much for this PR, maybe how about a compromise like allowing to use amd.id: true to mean: Just use the file name as id. Then we could still allow patterns at a later point if there is some demand without a breaking change.

lukastaegert · 2020-11-18T05:29:21Z

By the way, this is what a concatenation plugin could look like which you could use in a test:

{
  name: 'concatenate',
  generateBundle(options, bundle) {
  	if (options.format === 'amd') {
  		const concatenatedCode = Object.keys(bundle)
  			.map(chunkName => bundle[chunkName].code)
  			.join('\n');
  		for (const chunkName of Object.keys(bundle)) {
  			delete bundle[chunkName];
  		}
  		this.emitFile({ type: 'asset', fileName: 'main.js', source: concatenatedCode });
  	}
  }
}

It actually produced runnable code in your implementation if you set amd.id === "[id]".

tjenkinson · 2020-11-18T09:29:48Z

Ah I think I understand now.

But to make it work, one has to switch to using AMD ids instead of file names for all imports

Is this the case though? It looks like on the repl (and what I'm seeing in our app) that the imports are all relative ('./maths-0ec2b017)?

I think (but maybe misunderstood) you're suggesting if we were prepending rollup-bundle/ to the ids then we would need to update

new Promise(function (resolve, reject) { require(['./maths-0ec2b017'], resolve, reject) })

to

new Promise(function (resolve, reject) { require(['rollup-bundle/maths-0ec2b017'], resolve, reject) })

?

If you think this is too much for this PR, maybe how about a compromise like allowing to use amd.id: true to mean: Just use the file name as id. Then we could still allow patterns at a later point if there is some demand without a breaking change.

In our use case we need to support the pattern/prefix.

Also if requirejs requires the part after the final / to match the file name (without .js), I'm not sure a pattern makes sense? Maybe to make it clearer we could add amd.idPrefix and deprecate amd.id? Not sure if anyone would be depending on amd.id continuing to work the way it does now. If they're not, then amd.id could become an alias for amd.idPrefix?

lukastaegert · 2020-11-18T10:52:18Z

then we would need to update

Exactly

Also if requirejs requires the part after the final / to match the file name

Well, it is like this:

If the module is defined in a separate file, then requirejs will only find it if the WHOLE id matches the path + file name. Adding a prefix will break this in any case UNLESS in the final deployment, the entry point is not located at baseUrl but at a nested path and the prefix is that nested path. On the other hand
If the module definition is concatenated into the entry point, then the id can be a completely arbitrary string and it will still work

So these are two very distinct use cases. On top of that, the first use-case would still work with relative imports while the second would require full id imports.

So maybe in the end, maybe we also need to define if we use relative imports or absolute imports. But going with the absolute minimum that would keep the future open, maybe

amd: {
  id: true // use automatic ids,
  idPrefix: 'some/string' // just some string that is prefixed to the id
}

with the option to extend this to

amd: {
  id: ... // string or pattern string or function
  idPrefix: 'some/string' // as before, will be prepended to any id
  importById: false // use relative imports, set this to `true` to instead import by id
}

tjenkinson · 2020-11-18T12:09:14Z

Adding a prefix will break this in any case UNLESS in the final deployment, the entry point is not located at baseUrl but at a nested path and the prefix is that nested path

This is exactly our use case at the moment

If the module definition is concatenated into the entry point, then the id can be a completely arbitrary string and it will still work

Yep, but I'm wondering why someone would want arbitrary strings?

the first use-case would still work with relative imports while the second would require full id imports.

ah right. Hadn't noticed this case. It doesn't exist for us right now because the dynamic chunks aren't concat'd into the entry file.

amd: {
  id: true // use automatic ids,
  idPrefix: 'some/string' // just some string that is prefixed to the id
}

So with this config (id === true), we should also not use relative imports, but import everything by the complete id?

timiyay · 2020-11-23T00:18:51Z

This is very nice work! It looks like it might solve the need I had in #3333.

I tested it out with our codebase, using autoId and basePath. It all worked smoothly.

One consideration I wanted to report, to see what you think about it, relates to AMD module IDs and dependencies.

We have a situation where we have files that build 1 JS file per entrypoint (there can be a variable amount of entrypoints), and 2 shared JS files called vendor.js and common.js (configured as manualChunks).

If I build with this PR with config:

const outputOptions = {
  format: 'amd',
  amd: {
    autoId: true,
    basePath: 'foo'
  }
}

...then an entrypoint bundle called bar.js will involve the following modules:

// vendor.js
define('foo/vendor', [], function () {})

// common.js
define('foo/common', ['./vendor'], function (vendor) {})

// bar.js
define('foo/bar', ['./vendor', './common'], function (vendor, common) {})

In this case, the AMD ID isn't being used to import dependencies like ./vendor. They're still using a relative path. I'll need to refresh my memory on the limitations of the AMD module loader we're using (https://github.com/ember-cli/loader.js), but I'm not sure we can use relative paths.

Is there a use case for using the AMD module IDs, not the relative paths, for importing dependencies?

// vendor.js
define('foo/vendor', [], function () {})

// common.js
define('foo/common', ['foo/vendor'], function (vendor) {})

// bar.js
define('foo/bar', ['foo/vendor', 'foo/common'], function (vendor, common) {})

lukastaegert · 2020-11-23T05:03:25Z

As I already noted above, I did some experiments, the results were:

If the AMD id correctly matches the path of the file, you can still use relative imports and it will work
If it does not match
1. you need to use the id to run and import the AMD code
2. loading the file before you import it needs to be handled by yourself, e.g. by concatenating its contents into the entry point

Not sure if this answers your question.

tjenkinson · 2020-11-23T12:08:50Z

I'll need to refresh my memory on the limitations of the AMD module loader we're using (https://github.com/ember-cli/loader.js), but I'm not sure we can use relative paths.

One of the examples does require('./apple') so it looks like it would.

Is there a use case for using the AMD module IDs, not the relative paths, for importing dependencies?

Yeh like Lukas mentioned earlier, but this adds a lot more complexity and usually takes up more space, so we were thinking that could be an additional option importById in the future, if needed.

timiyay · 2020-11-23T21:36:48Z

Thanks for the replies, I'll give it some more testing.

As I recall, one of the issues I hit was the usage of a single AMD namespace for everything, so module names like vendor and common could collide. Importing by a custom AMD id like @scope/vendor solved this problem.

This problem was solved for production builds by including the fingerprinted module filename in its AMD ID, which fits a process where 1 file = 1 AMD module.

lukastaegert

This looks good and I agree with your suggestions. Beyond what I wrote in the comments, here are some other final points to address:

It would be really nice to have an error when the amd.id is used in a multi-file build. The best place to throw such an error IMO is in the validateOptionsForMultiChunkOutput function in Bundle.ts.
Coverage could still be improved in some places, see the codecov report:
- extracting how pattern functions are handled means that they now need to be tested for all three cases if they are correctly applied. It appears to be untested for entryFileNames. Maybe some existing test could just be extended to accommodate this.
- the error cases in normalizeOutputOptions are not tested. I know this is kind of a chore, they are usually done using simple function tests.

Thanks a lot for your hard work, this is a really great feature!

src/rollup/types.d.ts

docs/999-big-list-of-options.md

lukastaegert · 2020-11-26T06:05:24Z

src/Chunk.ts

@@ -712,13 +710,18 @@ export default class Chunk {
 			});
 		}

+		if (!this.id) {


Not a big fan of internal errors that are impossible to test, but there are other opinions out there as well. The price of course are non-null assertions. In a future version of Rollup, maybe we can move away from classes towards data types that evolve as the bundle evolves.

Yeh I had that initially but noticed a few more internal errors. Happy to remove if you want

maybe we can move away from classes towards data types that evolve as the bundle evolves.

sounds good

tjenkinson · 2020-11-27T15:11:32Z

It would be really nice to have an error when the amd.id is used in a multi-file build

Done, but I'm wondering if this would be breaking? Right now we set amd.id to !<< AMD ID PLACEHOLDER >>! and replace it in a plugin as a workaround. We're going to remove that after this is merged so it doesn't bother me either way.

Think I've addressed everything now except the coverage. Will do that now

tjenkinson · 2020-11-27T16:20:19Z

I think the only reason coverage is down in chunk.ts now is because of the internal error. We could add a /* istanbul ignore next */ above it, or remove that check? or maybe it's ok

tjenkinson · 2020-11-27T16:22:26Z

added the comment because saw the same is done for other internal errors.

lukastaegert · 2020-11-28T06:05:11Z

Done, but I'm wondering if this would be breaking? Right now we set amd.id to !<< AMD ID PLACEHOLDER >>! and replace it in a plugin as a workaround. We're going to remove that after this is merged so it doesn't bother me either way.

You are very right! I am sorry I did not even consider this could happen. Then we should make the error a warning.

lukastaegert

Ok, just the warning/error, then we are good to go from my side.

lukastaegert · 2020-11-28T06:11:06Z

test/typescript/index.ts

+	{
+		basePath: ''
+	}
+];


src/Bundle.ts

implement amd.idFromChunkName option

cef70d8

tjenkinson commented Nov 13, 2020

View reviewed changes

update output-options-hook test

a44bef3

tjenkinson marked this pull request as ready for review November 13, 2020 16:17

Tom Jenkinson added 5 commits November 13, 2020 16:19

update NormalizedOutputOptions type

eadea09

simiplify removeExtension()

462c69c

fix idFromChunkName type

287769a

make idFromChunkName required in normalized options

65f0921

reference actual types in normalizeOutputOptions

3a42519

lukastaegert reviewed Nov 15, 2020

View reviewed changes

switch to using a pattern instead

04ec70f

tjenkinson added 6 commits November 15, 2020 15:08

also removeExtensionFromRelativeAmdId in UMD finaliser

3bcd1cd

Merge branch 'remove-extension-amd-umd' into amd-id-code-split

02b8f21

support id in output.amd.id in umd finaliser

2e9d487

move test to chunking-form/samples

cb76695

remove test from cli section

881b932

remove idFromChunkName

e3cf09c

tjenkinson force-pushed the amd-id-code-split branch from 7127af0 to e3cf09c Compare November 15, 2020 15:55

lukastaegert reviewed Nov 16, 2020

View reviewed changes

lukastaegert reviewed Nov 17, 2020

View reviewed changes

Tom Jenkinson added 2 commits November 20, 2020 15:08

update docs

114364f

update test to also put chunks in folder

9434622

lukastaegert requested changes Nov 26, 2020

View reviewed changes

Tom Jenkinson added 3 commits November 27, 2020 14:24

throw error if amd.id used with muli-file build

8eeece1

more accurate amd options type

1654ded

update output.amd doc

1d4efc9

Tom Jenkinson added 3 commits November 27, 2020 15:24

fix @ts-expect-error comments

84e8099

add tests for invalid options

96ea9c1

add test for entryFileNames with output.preserveModules

c9ec971

tjenkinson changed the title ~~implement amd.idFromChunkName option~~ implement amd.autoId/amd.basePath options Nov 27, 2020

tjenkinson requested a review from lukastaegert November 27, 2020 16:20

Tom Jenkinson added 6 commits November 27, 2020 16:22

ignore internal error in coverage

48d01b1

simplify type

e1d9173

fix output options test

e6944ae

fix type tests

9cecce3

improve types again

6ef8520

simply again

bba3d17

lukastaegert requested changes Nov 28, 2020

View reviewed changes

test/typescript/index.ts

{

basePath: ''

}

];

Copy link

Member

lukastaegert Nov 28, 2020

Choose a reason for hiding this comment

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

👍

src/Bundle.ts Show resolved Hide resolved

make it a warning

cbae7b8

tjenkinson requested a review from lukastaegert November 28, 2020 12:06

lukastaegert approved these changes Nov 29, 2020

View reviewed changes

lukastaegert merged commit 163252a into rollup:master Nov 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement `amd.autoId`/`amd.basePath` options #3867

implement `amd.autoId`/`amd.basePath` options #3867

tjenkinson commented Nov 13, 2020 •

edited

tjenkinson Nov 13, 2020

lukastaegert Nov 15, 2020

codecov bot commented Nov 13, 2020 •

edited

lukastaegert left a comment

lukastaegert Nov 15, 2020

lukastaegert Nov 15, 2020

tjenkinson commented Nov 15, 2020

tjenkinson commented Nov 15, 2020 •

edited

lukastaegert Nov 16, 2020

lukastaegert Nov 16, 2020

lukastaegert commented Nov 17, 2020

lukastaegert Nov 17, 2020

lukastaegert Nov 17, 2020

tjenkinson commented Nov 17, 2020

lukastaegert commented Nov 18, 2020

lukastaegert commented Nov 18, 2020

tjenkinson commented Nov 18, 2020

lukastaegert commented Nov 18, 2020

tjenkinson commented Nov 18, 2020 •

edited

timiyay commented Nov 23, 2020 •

edited

lukastaegert commented Nov 23, 2020

tjenkinson commented Nov 23, 2020

timiyay commented Nov 23, 2020 •

edited

lukastaegert left a comment •

edited

lukastaegert Nov 26, 2020

tjenkinson Nov 27, 2020

tjenkinson commented Nov 27, 2020

tjenkinson commented Nov 27, 2020

tjenkinson commented Nov 27, 2020

lukastaegert commented Nov 28, 2020

lukastaegert left a comment

lukastaegert Nov 28, 2020

implement amd.autoId/amd.basePath options #3867

implement amd.autoId/amd.basePath options #3867

Conversation

tjenkinson commented Nov 13, 2020 • edited

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 13, 2020 • edited

Codecov Report

lukastaegert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjenkinson commented Nov 15, 2020

tjenkinson commented Nov 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukastaegert commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjenkinson commented Nov 17, 2020

lukastaegert commented Nov 18, 2020

lukastaegert commented Nov 18, 2020

tjenkinson commented Nov 18, 2020

lukastaegert commented Nov 18, 2020

tjenkinson commented Nov 18, 2020 • edited

timiyay commented Nov 23, 2020 • edited

lukastaegert commented Nov 23, 2020

tjenkinson commented Nov 23, 2020

timiyay commented Nov 23, 2020 • edited

lukastaegert left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjenkinson commented Nov 27, 2020

tjenkinson commented Nov 27, 2020

tjenkinson commented Nov 27, 2020

lukastaegert commented Nov 28, 2020

lukastaegert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

implement `amd.autoId`/`amd.basePath` options #3867

implement `amd.autoId`/`amd.basePath` options #3867

tjenkinson commented Nov 13, 2020 •

edited

codecov bot commented Nov 13, 2020 •

edited

tjenkinson commented Nov 15, 2020 •

edited

tjenkinson commented Nov 18, 2020 •

edited

timiyay commented Nov 23, 2020 •

edited

timiyay commented Nov 23, 2020 •

edited

lukastaegert left a comment •

edited