Skip to content

Conversation

wkeese
Copy link

@wkeese wkeese commented Feb 11, 2020

Here are some proposed changes for es6.js to be able to load lit-html (and other problematic modules). The rest of the changed files are just for testing and don't necessarily need to be checked in.

The problem I saw was with loading lit-html.js via an absolute path, and then lit-html.js tried to load files relative to itself via relative paths like "./lib/default-template-processor.js".

The first issue is that es6 appends ".js" even though the path already has an extension. So that's a trivial one-line fix.

The second problem is to fix relative paths. es6 was interpreting the relative path above to be relative to the original module, or something like that. I updated resolvePath() so it interprets it relative to the file doing the import.

I tested on chrome and IE11. Didn't try a build though, since we aren't using r.js for builds anymore.

Fixes #34.

and that relative paths within the original example
work correctly.

You probably don't want to commit this to the repository,
or maybe want to commit it as another demo/test.

Also, note that I couldn't support a path for lit-html of

'lit-html': '../node_modules/lit-html/lit-html'

because then there's no way to specify the path to lit-html's support
files (i.e. the other files in the lit-html/ directory).  So instead
I did:

'lit-html': '../node_modules/lit-html'
@prantlf
Copy link
Contributor

prantlf commented Jan 2, 2022

The .js extension could be really appended once more to the name, although it already contained it. However, your fix preventing the appending if the name starts with ./ or ../ is not proper. The name parameter passed to the plugin is not the original module name from the source code. For example:

import from 'src/sum'  ==> the plugin gets 'src/sum'
import from './sum'    ==> the plugin gets 'src/sum'
import from './sum.js' ==> the plugin gets 'src/sum.js'

RequireJS does not allow appending .js to module names from not relative paths. However, when writing a plugin, the ./ is not passed to it. As a workaround, I check if the name does not end with .js. Not the ultimate fix, but works probably well enough.

Loading lit-html needs no fix in my example.

@wkeese
Copy link
Author

wkeese commented Jan 4, 2022

OK, thanks for looking at it.

However, your fix preventing the appending if the name starts with ./ or ../ is not proper. The name parameter passed to the plugin is not the original module name from the source code.

OK, my fix does work for my test case (you can confirm it from my branch), but your fix in 8f93a0e looks like it works too (although I prefer more concise code like .replace(/\.js$/, "")).

Loading lit-html needs no fix in my example.

OK. I'm unclear how your example is working given that you define the path to lit-html as

'lit-html': '../node_modules/lit-html/lit-html'

which presumably points to node_modules/lit-html/lit-html.js, but then somehow you do

import { html, render } from 'lit-html';

which somehow loads node_modules/lit-html/lib/html.js and node_modules/lit-html/lib/render.js.

Having said that, I can't reproduce the relative path problem I saw a few years ago, so I guess resolvePath() doesn't need to be changed after all.

@wkeese
Copy link
Author

wkeese commented Jan 4, 2022

PS: Oh, your demo is using lit-html 2.0.2 whereas my test file is using lit-html 1.1.2. In lit-html 2.0.2, lit-html.js is self-contained, it doesn't do a

import { defaultTemplateProcessor } from './lib/default-template-processor.js';

etc. So, your demo isn't even testing relative paths like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

path and file extension errors loading lit-html
2 participants