This is the file for 3.8.1 except that it doesn't actually work

[michael-simon]

Riot has two pieces of code that need to be updated before JSPM will run out of the box with this json file, and I have no idea how to resolve them, but this at least lets me start the discussion.

Riot does not have relative pathing in its lib/server/index.js, it has an absolute pathing, and that doesn't work with deps in any way I could reasonably find. I have a patch out with Riot to fix that under the assumption that's necessary. There's also another issue where they use require.extensions which doesn't work with SystemJS around, and that's in my same patch for them.

[aluanhaddad]

@michael-simon Greetings!

Thanks for the PR.

I have a couple of questions

Firstly, which version of JSPM are you using?

$ jspm --version

Secondly, can you clarify the following remark?

Riot does not have relative pathing in its lib/server/index.js, it has an absolute pathing, and that doesn't work with deps in any way I could reasonably find.

I don't have experience with Riot, but packages which do not use relative paths to import from within their own package are fully supported.

Finally

There's also another issue where they use require.extensions which doesn't work with SystemJS around, and that's in my same patch for them.

Can you point me to where I need to see this in their code?

[michael-simon]

@aluanhaddad

• I've done this under two versions of JSPM 0.17: 0.17.0-beta.47 and 42 and in both cases it works the same.
• So in detail, https://github.com/riot/riot/blob/6b5225fe9e58deb94d9f714ba74276b536733702/lib/server/index.js#L7 is the line in question: they use this to determine the require they then invoke. However, in the deps[] section in package.json, if I use ../../riot, SystemJS does not see the System.registerDynamic as satisfied for FULLPATH/riot . If I instead use path.join('..','..','riot') for the second part of the line, then SystemJS is fine. If there's a way to get this deps statement to work without changing their code, great, but I couldn't figure one.
• Sure, so this expresses itself by, once you get pathing working, saying that require.extensions is null on line https://github.com/riot/riot/blob/6b5225fe9e58deb94d9f714ba74276b536733702/lib/server/index.js#L54 . The only solution that immediately occurred to me was to check if require.extensions even exists. The 'require' that exists at that point seems to be the SystemJS require, which apparently doesn't have extensions. I did this in a minimal test case so nothing else but Riot and JSPM would be in the way.

[aluanhaddad]

Thanks for the clarification. Took a look at your test repo as well. and a bit at Riot in the meantime.

They certainly do some tricky things, that is for sure.

For example, they have a reference to own of their own dependencies that is qualified with nodule_modules.

  "bin": {
    "riot": "node_modules/riot-cli/lib/index.js"
  },
  "main": "lib/server/index.js",
  "module": "lib/riot.js",
  "jsnext:main": "lib/riot.js",
  "browser": "riot.js"

What is the use case for running lib/server/index.js in a browser? The package.json seems to assume that the "main" will be read by NodeJS and and that this will not be a problem because Webpack will prefer "module" and Rollup will look at "jsnext:main".

Anyway, when I run your repro against a version install either with or without this override, I get an error that riot does not have a render function.

The library does ship self contained browser builds, could they meet your needs or are you looking to leverage Rollup optimizations in JSPM. Either is possible.

[aluanhaddad]

Also https://github.com/riot/riot/blob/6b5225fe9e58deb94d9f714ba74276b536733702/lib/server/index.js#L16

I don't like this line at all

[michael-simon]

Yeah, I get why they're doing it (we actually have significant problems with it in JSPM if we're trying to bundle for that, at least, I did the first time I Was trying stuff out), but that delete line is kinda terrifying.

I remember running into riot.render not existing, trying to remember when. I assume you're doing jspm run test.js ? I remember having the riot.render not existing problem when I was trying to bundle myself.

Edit: Blowing away my directory and running my readme.md instructions for both the master (broken) and JSPM17Upgrade (fixed) versions from a clean clone resolved fine. Mmm.

[aluanhaddad]

No, I wasn't doing JSPM run test sorry apparently I cannot read.

[michael-simon]

Also, as a matter of clarity, this is all for serverside code, the browser side stuff is working completely fine as far as I can tell. We're trying to put together an isomorphic app.

[aluanhaddad]

Also, as a matter of clarity, this is all for serverside code, the browser side stuff is working completely fine as far as I can tell. We're trying to put together an isomorphic app.

Thank you ❤️. Now it all makes sense!

When you add or update overrides add the -f (--force) flag to jspm install like

$ jspm install riot -f -o ./override.json

[aluanhaddad]

I am going to hold off on merging this for the moment until we can get @guybedford's take on support for this scenario. I lack experience with isomorphic rendering.

[michael-simon]

I updated the repository to make the override match Riot's released version that has the necessary code fixes: 3.9.0 .

Also, the 'browser' tag in the fix is what's supposed to deal with the isomorphism issue: in the browser it treats an attempt to access the base package of lib/server/index.js as getting riot.js instead. (That part of the code is what JSPM 0.17.42 and 49 both produced by default, my additions were related to the relative code path issue in lib/server/index.js.)