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

Rename define to defineComponent #340

Merged
merged 1 commit into from Feb 12, 2015

Conversation

c0
Copy link
Contributor

@c0 c0 commented Feb 10, 2015

  • Rename define to defineComponent in component.js

component.js's internal define function can be mixed up with AMD's use of define as seen in the amd-optimize bug.

Background

I ran across an error when using amd-optimize where it incorrectly thought that internal function called define was a request for AMD dependencies. I've opened a PR there but have not received word that it will be merged/considered.

My thought is that changing Flight will make it more flexible and not use the "reserved word" define.

Feedback welcomed.

- Rename `define` to `defineComponent` in `component.js`

`component.js`'s internal `define` function can be mixed up with
AMD's use of `define` as seen in  [amd-optimize](scalableminds/amd-optimize#42)
@c0 c0 mentioned this pull request Feb 10, 2015
@tgvashworth
Copy link
Contributor

Thanks very much for moving this over.

So now the question is: do we merge this or wait for amd-optimize to fix their bug?

@c0
Copy link
Contributor Author

c0 commented Feb 10, 2015

I wonder if only this one should be merged. I'm not fully versed in the AMD spec, but is define essentially a reserved word? If so, this PR is applicable regardless of changes to amd-optimize.

@tgvashworth
Copy link
Contributor

That's a good question. It's possible that we'll move to CommonJS for v2 but for now this seems good enough to prevent these errors.

Will merge now but I probably won't get a release out until next week.

tgvashworth added a commit that referenced this pull request Feb 12, 2015
@tgvashworth tgvashworth merged commit 3cc137f into flightjs:v1.x Feb 12, 2015
@c0 c0 deleted the cw/rename_define_1_x branch February 12, 2015 15:13
@c0
Copy link
Contributor Author

c0 commented Feb 12, 2015

@c0 c0 restored the cw/rename_define_1_x branch February 23, 2015 15:12
@c0 c0 deleted the cw/rename_define_1_x branch April 22, 2015 15:58
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.

None yet

2 participants