Extend the existing import-map when using htmlGenerate #117
Conversation
|
|
|
Extending the existing + actual - expected ... Lines skipped
'\n' +
'<!doctype html>\n' +
...
' </script>\n' +
' <link rel="modulepreload" href="/react.js" />\n' +
+ ' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/cjs/react.production.min.js" />\n' +
' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/index.js" />\n' +
'<script type="module">\n' +
" import 'react';\n" +
'</script>\n'
at file:///Users/jkrishna/Documents/jspm/generator/test/html/indent.test.js:13:8 {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: '\n' +
'<!doctype html>\n' +
' <!-- Generated by @jspm/generator - https://github.com/jspm/generator -->\n' +
' <script async src="https://ga.jspm.io/npm:es-module-shims@1.5.1/dist/es-module-shims.js" crossorigin="anonymous"></script>\n' +
' <script type="importmap">\n' +
' {\n' +
' "imports": {\n' +
' "object-assign": "/react.js",\n' +
' "react": "https://ga.jspm.io/npm:react@17.0.2/index.js"\n' +
' }\n' +
' }\n' +
' </script>\n' +
' <link rel="modulepreload" href="/react.js" />\n' +
' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/cjs/react.production.min.js" />\n' +
' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/index.js" />\n' +
'<script type="module">\n' +
" import 'react';\n" +
'</script>\n',
expected: '\n' +
'<!doctype html>\n' +
' <!-- Generated by @jspm/generator - https://github.com/jspm/generator -->\n' +
' <script async src="https://ga.jspm.io/npm:es-module-shims@1.5.1/dist/es-module-shims.js" crossorigin="anonymous"></script>\n' +
' <script type="importmap">\n' +
' {\n' +
' "imports": {\n' +
' "object-assign": "/react.js",\n' +
' "react": "https://ga.jspm.io/npm:react@17.0.2/index.js"\n' +
' }\n' +
' }\n' +
' </script>\n' +
' <link rel="modulepreload" href="/react.js" />\n' +
' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/index.js" />\n' +
'<script type="module">\n' +
" import 'react';\n" +
'</script>\n',
operator: 'strictEqual'
}We can see a additional |
|
Can you please provide a test case that shows what this is solving in the first place? |
|
Thanks for adding a test case... what exactly is the issue that the test case doesn't work on current generator? It seems like it should? |
|
It works, but starting to break other tests. Before trying to fix them, I would like to know if this can be a valid use case to solve. |
|
@JayaKrishnaNamburu I mean what is the issue that this PR solves? Looking at the test I can't tell why this test fails against the main branch. |
|
@JayaKrishnaNamburu specifically I'm thinking here - I know of at least two bugs that might cause generation issues with versions and existing maps which I could fix, so knowing the test case and why it's failing in the first place would help to know if this is one of those bugs or indeed a new feature request. |
|
It's the test from <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/cjs/react.production.min.js" />\n' +
' <link rel="modulepreload" href="https://ga.jspm.io/npm:react@17.0.2/index.js" />\n' +It's adding two module preloads instead of just one. Where the import-map is just {
"imports": {
"object-assign": "/react.js"
}
} |
|
It would really help for communication to always include a clear description and test when creating a PR to begin with. I'm happy to help, I'm just very short on time. I'll take a look in the next couple of days. |
|
Just cloned and ran this which was necessary to get the context, and I finally get the concept that you want to retain the existing map. Is this because you have a set import map of possible packages that must be retained for dynamic imports that are untraceable by the generator? If so, do you need to use the map as the source of truth or would it be possible to just rely on |
|
I feel, replying on pre-defined |
|
I see, thanks, yes I agree So the reason for the test failures then is that they share the generator instance, and thus Let's rather store |
Yes, exactly what i was trying to say before 😅 |
I see, but you did not mention |
|
This should now be fully supported by #134. |
No description provided.