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.

Sign up for GitHub

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

Jump to bottom

Make RequireJS work with can.import #1456

Closed
matthewp opened this issue Feb 18, 2015 · 8 comments
Closed

Make RequireJS work with can.import #1456

matthewp opened this issue Feb 18, 2015 · 8 comments
Assignees
@matthewp
Labels
enhancement
Milestone
2.2.0

Comments

@matthewp
Copy link
Contributor

No description provided.

@matthewp matthewp self-assigned this Feb 18, 2015
@matthewp matthewp added this to the 2.2.0 milestone Feb 18, 2015
@matthewp
Copy link
Contributor Author

Looks like there is already code: https://github.com/bitovi/canjs/pull/1396/files#diff-95ad04d772620344cfa50f85ba7adb64R96

I guess we just need a test to verify it is working?

@justinbmeyer
Copy link
Contributor

It doesn't work.

Sent from my iPhone

On Feb 19, 2015, at 6:00 AM, Matthew Phillips notifications@github.com wrote:

Looks like there is already code: https://github.com/bitovi/canjs/pull/1396/files#diff-95ad04d772620344cfa50f85ba7adb64R96

I guess we just need a test to verify it is working?

—
Reply to this email directly or view it on GitHub.

@matthewp
Copy link
Contributor Author

Interesting, is there an existing demo page or something to test with? Otherwise I'll create one.

@justinbmeyer
Copy link
Contributor

There's a test page for steal's integration.

Sent from my iPhone

On Feb 19, 2015, at 7:46 AM, Matthew Phillips notifications@github.com wrote:

Interesting, is there an existing demo page or something to test with? Otherwise I'll create one.

—
Reply to this email directly or view it on GitHub.

@matthewp
Copy link
Contributor Author

One issue is that we are importing can/view/stache/stache which for amd it should just be can/view/stache. This is only true of amd so I think we can just denormalize it in this one place.

@matthewp
Copy link
Contributor Author

I really don't like doing an amd check in autorender but can't think of a better way.

@justinbmeyer
Copy link
Contributor

Yeah, this will be gone in 3.0

Sent from my iPhone

On Feb 19, 2015, at 9:01 AM, Matthew Phillips notifications@github.com wrote:

I really don't like doing an amd check in autorender but can't think of a better way.

—
Reply to this email directly or view it on GitHub.

matthewp added a commit to matthewp/canjs that referenced this issue Feb 19, 2015
@matthewp
can.import works with RequireJS
27fc48e 
This fixes can.import to work with RequireJS. 2 things needed:

1. can.import checks for define && define.amd instead of window.require
(window.require.amd doesn't exist).

2. can.autorender has to pass in the correct moduleName for AMD.

Fixes canjs#1456
@matthewp matthewp mentioned this issue Feb 19, 2015
Merged
@daffl
Copy link
Contributor

daffl commented Feb 19, 2015

Closed via #1460

@daffl daffl closed this as completed Feb 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewp matthewp

Labels
enhancement
Projects
None yet
Milestone
2.2.0
Development

No branches or pull requests
3 participants
@justinbmeyer @daffl @matthewp