Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update UMD to have an export #5

Closed
wants to merge 2 commits into from
Closed

Update UMD to have an export #5

wants to merge 2 commits into from

Conversation

JonathanWolfe
Copy link

https://github.com/umdjs/umd/blob/6e693154138b111e17b264e4e27f76e6116f1aba/returnExports.js#L17-L37

Update the UMD to an export variant to allow for node require-ing of pre-compiled tags that used the --modular flag.

@GianlucaGuarini
Copy link
Member

@JonathanWolfe thank you for your pull request I see you have removed the require, export, module variables but they were used to solve riot/riot#1044 Is there a reason you want to have an export for node? riot.tag2 does not return anything special

@JonathanWolfe
Copy link
Author

About riot/riot#1044 - No, I didn't realize it was used for anything.

The problem I'm facing is that node pre-rendering doesn't work (anymore?) because when require-ing the .tag it fails to compile because the node compiler doesn't require('riot').

const riot = require('riot');
const tag = require('./example.tag');

riot.render( example, { foo:'bar'} )

Returns:

ReferenceError: riot is not defined
    at Object.<anonymous> (/Users/jonwolfe/repos/hopspot/src/templates/pages/app.tag:1:80)
    at Module._compile (module.js:425:26)
    at Object.require.extensions..tag (/Users/jonwolfe/repos/hopspot/node_modules/riot/lib/server/index.js:14:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at loadSite (/Users/jonwolfe/repos/hopspot/server/prerender.js:33:20)
   at ...express stuff...

But if you pre-compile the tags to .js (with modular flag) then require them you get:

TypeError: tagName.toUpperCase is not a function
    at new simple$dom$document$element$$Element (/Users/jonwolfe/repos/hopspot/node_modules/simple-dom/dist/simple-dom.js:161:25)
    at simple$dom$document$$Document.createElement (/Users/jonwolfe/repos/hopspot/node_modules/simple-dom/dist/simple-dom.js:299:14)
    at createTag (/Users/jonwolfe/repos/hopspot/node_modules/riot/lib/server/index.js:21:23)
    at Object.riot.render (/Users/jonwolfe/repos/hopspot/node_modules/riot/lib/server/index.js:27:13)
    at loadSite (/Users/jonwolfe/repos/hopspot/server/prerender.js:36:24)
    at ...express stuff...

Because the returned value of the node require statement is {} not the tag because the module doesn't export anything.

So if you add an export the UMD and return the tag statement, you'll get back a tag that riot.render can then process.

@GianlucaGuarini
Copy link
Member

I have already fixed it ;) riot/riot#1307

@JonathanWolfe
Copy link
Author

Oh. Well you can do whatever you want with this PR then :)

@GianlucaGuarini
Copy link
Member

😄 @JonathanWolfe thanks anyway we have always 1000 things to do and it's always good if other devs like you could help us on the project. I will close this issue and thanks again for testing the beta

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.

2 participants