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

Feature/convert to npm #89

Open
wants to merge 113 commits into
base: develop
Choose a base branch
from
Open

Feature/convert to npm #89

wants to merge 113 commits into from

Conversation

qejk
Copy link
Member

@qejk qejk commented Jan 14, 2017

Will review code again to get all quirks here commented in few days.

qejk added 30 commits April 19, 2016 06:39
… multiple logging adapters in Space.Logger, adds adapter for console, winston logging libraries, wires everything up in module.coffee
…ds setMinLevel, adds removable transports at winston adapter
…eature/logger-refactor

# Conflicts:
#	package.js
// Publishes a module into the space environment to make it
// visible and requireable for other modules and the application
publish(module, identifier) {
// TODO: its overriding name necessary?
Copy link
Member Author

Choose a reason for hiding this comment

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

This property can't be assigned do to TypeError: Cannot assign to read only property 'name' of function 'function GrandchildModule() { initialize.apply(this, arguments); }', unclear about approach here.

version: 0.10.33
pre:
- curl https://install.meteor.com | /bin/sh
version: 6.0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

Do to:

    for (let [index, struct] of ObjectEntries(actual)) {
                             ^^

SyntaxError: Unexpected identifier

Can't figure out why this works above 6.0.0, tried 4.0.0, 5.0.0 and one from 0.10.* - no luck. This should work on those versions. babel/babel#5041


// @backward {space:base} <= 4.1.3 for Meteor package,
// Give every module access Npm
if (!isNil(Meteor) && this._isServer() && !isNil(Npm)) {
Copy link
Member Author

@qejk qejk Jan 14, 2017

Choose a reason for hiding this comment

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

I think this could be extracted to predefined dependencies. Such, would be added via static method on Module.addPredefinedDependency/Module.setPredefinedDependencisthat would allow space-base be framework agnostic - however still, allow to add separate framework-oriented dependencies with all sugary goodnes out of the box.

This would result in Meteor adding default mappings like it was done previously here:

  _mapMeteorApis: ->
    @injector.map('Meteor').to Meteor
    if Package.ejson?
      @injector.map('EJSON').to Package.ejson.EJSON
    if Package.ddp?
      @injector.map('DDP').to Package.ddp.DDP
    if Package.random?
      @injector.map('Random').to Package.random.Random
    @injector.map('underscore').to Package.underscore._
    if Package.mongo?
      @injector.map('Mongo').to Package.mongo.Mongo
      if Meteor.isServer
        @injector.map('MongoInternals').to Package.mongo.MongoInternals

    if Meteor.isClient
      if Package.tracker?
        @injector.map('Tracker').to Package.tracker.Tracker
      if Package.templating?
        @injector.map('Template').to Package.templating.Template
      if Package.session?
        @injector.map('Session').to Package.session.Session
      if Package.blaze?
        @injector.map('Blaze').to Package.blaze.Blaze

    if Meteor.isServer
      @injector.map('check').to check
      @injector.map('Match').to Match
      @injector.map('process').to process
      @injector.map('Future').to Npm.require 'fibers/future'

      if Package.email?
        @injector.map('Email').to Package.email.Email

    if Package['accounts-base']?
      @injector.map('Accounts').to Package['accounts-base'].Accounts

    if Package['reactive-var']?
      @injector.map('ReactiveVar').to Package['reactive-var'].ReactiveVar```

this.injector.map('log').to(this.log);
},

_isServer() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Ensure that this is working as intended with webpack on client and server.

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

Successfully merging this pull request may close these issues.

4 participants