Skip to content
New issue

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

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

Already on GitHub? # to your account

Undefining a module no longer undefines special modules, issue #128 #130

Merged
merged 1 commit into from
May 5, 2017

Conversation

rorticus
Copy link
Contributor

Type: bug

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Fix for undefining modules that have dependencies on require, exports, or module. These special modules should never be undefined!

Resolves #128

@rorticus rorticus requested a review from kitsonk April 28, 2017 15:10
@codecov
Copy link

codecov bot commented Apr 28, 2017

Codecov Report

Merging #130 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   85.58%   85.63%   +0.05%     
==========================================
  Files           1        1              
  Lines         548      550       +2     
  Branches      136      137       +1     
==========================================
+ Hits          469      471       +2     
  Misses         32       32              
  Partials       47       47
Impacted Files Coverage Δ
_build/src/loader.ts 85.63% <0%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cb0fcf...b619a26. Read the comment docs.

Copy link
Member

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

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

Only thought I have, do we want to preserve/guard ourselves against someone doing require.undef('require')? Maybe that is just being paranoid.

@rorticus
Copy link
Contributor Author

Yeah I thought about that. I'm not sure where the line is on how much we should be protect people from doing something ... questionable.

@kitsonk
Copy link
Member

kitsonk commented Apr 28, 2017

Yeah, I guess this meets the scope of the issue and we shouldn't allow me to scope creep us!

@dylans dylans modified the milestones: 2017.04, 2017.05 Apr 29, 2017
@rorticus rorticus merged commit 020f647 into dojo:master May 5, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants