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

Compiler.generate: support YModules, CommonJS and global dependencies #424

Merged
merged 1 commit into from
Mar 20, 2017

Conversation

miripiruni
Copy link
Contributor

@miripiruni miripiruni commented Mar 9, 2017

Fixes #375

  • Russian docs
  • English docs
  • Tests

@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage decreased (-0.4%) to 99.348% when pulling 1c3c665 on issue-375__modules into 4adfc67 on master.

lib/compiler.js Outdated

'})(typeof window !== "undefined" ? ' +
'window : typeof global !== "undefined" ? global : this);'

].join('\n');
Copy link
Contributor Author

@miripiruni miripiruni Mar 9, 2017

Choose a reason for hiding this comment

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

EOL = require('os').EOL

var vow = require('vow');
/* jscs:disable */
// fake commonJS module after browserify
var FAKE_COMMON_MODULE = 'var require=(function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module \'"+o+"\'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({"fake":[function(require,module,exports){ module.exports = { getText: function () { return "Hello templates!"; } }; },{}]},{},[]);';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

var bundle = bemhtml.generate('', {
requires: { text: { globals: 'text' } }
});
var sandbox = { global: {}, exports: {} };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Сделать метод как в enb-bemxjst

@miripiruni miripiruni force-pushed the issue-375__modules branch 4 times, most recently from e2043e2 to 02628c1 Compare March 16, 2017 14:18
@miripiruni miripiruni changed the title [WIP] Compiler.generate: support YModules, CommonJS and global dependencies Compiler.generate: support YModules, CommonJS and global dependencies Mar 16, 2017
@miripiruni miripiruni requested a review from qfox March 16, 2017 14:19
@coveralls
Copy link

coveralls commented Mar 16, 2017

Coverage Status

Coverage increased (+0.005%) to 99.762% when pulling 02628c1 on issue-375__modules into bbafe2c on master.

commonJSNames: [],
ym: [],
ymVars: [],
ymLibs: []
Copy link
Member

Choose a reason for hiding this comment

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

ym настолько особенный, что аж ну ваще

return deps;

for (var lib in requires) {
if (requires.hasOwnProperty(lib)) {
Copy link
Member

Choose a reason for hiding this comment

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

if (!requires.hasOwn...) continue; ?

@miripiruni
Copy link
Contributor Author

@zxqfox how is it?

@@ -456,6 +424,111 @@ $ cat stderr.txt
BEMXJST ERROR: cannot render block b1, elem undefined, mods {}, elemMods {} [TypeError: Cannot read property 'undef' of undefined]
```

### Using thirdparty libraries
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tadatuta can you check this english documentation?

@coveralls
Copy link

coveralls commented Mar 20, 2017

Coverage Status

Coverage increased (+0.007%) to 99.683% when pulling c4027be on issue-375__modules into 79a8522 on master.

Copy link
Member

@qfox qfox left a comment

Choose a reason for hiding this comment

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

Looked also at tests, looks fine.

'function buildBemXjst(libs) {',
'var exports;',

'/* BEM-XJST Runtime Start */',
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want replace these great beautiful banners with usual boring comment blocks?

) + '}'),

// Provide to YModules
'if (typeof modules === "object") {',
Copy link
Member

Choose a reason for hiding this comment

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

Afaik electron has modules but not ym. /cc @awinogradov

Copy link
Member

Choose a reason for hiding this comment

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

ym has modules 100%

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@qfox qfox Mar 20, 2017

Choose a reason for hiding this comment

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

I mean electron has no ym but has modules variable and this code will be broken there. AFAIK.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, the electron context is a product of merging browser and nodejs contexts, and we have there modules, module and require at the same time. I'd add a test for this.

),

'})(typeof window !== "undefined" ? ' +
'window : typeof global !== "undefined" ? global : this);'
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this code smell, but I don't atm know how to bake it better.

I'd say it's fine solution since compile should generate code with exports.

docs/en/3-api.md Outdated
}
```

`lib-name` module will accessible in templates body like this:
Copy link
Member

Choose a reason for hiding this comment

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

will accessible -> will be accessible

docs/en/3-api.md Outdated
});
```

It’s necessary to specify all the modular systems for connecting the library.
Copy link
Member

Choose a reason for hiding this comment

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

- It’s necessary to specify all the modular systems for connecting the library.
+ It’s necessary to specify each environment you want to expose library to.

docs/en/3-api.md Outdated

It’s necessary to specify all the modular systems for connecting the library.

For example: you can specify dependencies only from global scope. In this case
Copy link
Member

Choose a reason for hiding this comment

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

E.g. if you specify just global scope the library will only be available as global variable even though some module system will be present in runtime.

docs/en/3-api.md Outdated

Example of using `moment.js` library:

You need no set path to module:
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to to provide path to module

docs/ru/3-api.md Outdated

## Подключение сторонних библиотек

Технологии [bemtree](api.ru.md#bemtree) и [bemhtml](api.ru.md#bemhtml) поддерживают возможность подключения сторонних библиотек как глобально, так и для разных модульных систем с помощью опции [requires](api.ru.md#requires).
Copy link
Member

Choose a reason for hiding this comment

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

В английском было капсом, что правильно

docs/ru/3-api.md Outdated
@@ -451,6 +419,110 @@ BEMXJST ERROR: cannot render block b1, elem undefined, mods {}, elemMods {} [Typ
```


### Расширение BEMContext
Copy link
Member

Choose a reason for hiding this comment

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

порядок не совпадает с порядком в TOC-е

) + '}'),

// Provide to YModules
'if (typeof modules === "object") {',
Copy link
Member

Choose a reason for hiding this comment

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

ym has modules 100%

@miripiruni
Copy link
Contributor Author

@tadatuta updated!

@coveralls
Copy link

coveralls commented Mar 20, 2017

Coverage Status

Coverage increased (+0.007%) to 99.683% when pulling b20bb52 on issue-375__modules into 79a8522 on master.

@miripiruni miripiruni merged commit 45d8415 into master Mar 20, 2017
@miripiruni miripiruni deleted the issue-375__modules branch March 20, 2017 19:26
@qfox
Copy link
Member

qfox commented May 23, 2017

Я тут запустил, и у меня require('./generated.bemhtml.js') не работает :-( Кажется, CommonJS сломано

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.

4 participants